Ruby on Rails quiz function I have created a quiz function in my rails application. Since I want to track responses and make it a server-side application I created a table to record the responses of the questions people answer.

create_table "responses", force: :cascade do |t|
    t.bigint "question_id"
    t.bigint "answer_id"
    t.bigint "user_id"
    t.uuid "uuid", default: -> { "uuid_generate_v4()" }
    t.datetime "created_at", null: false
    t.datetime "updated_at", null: false
    t.index ["answer_id"], name: "index_responses_on_answer_id"
    t.index ["question_id"], name: "index_responses_on_question_id"
    t.index ["user_id"], name: "index_responses_on_user_id"
  end

Each question can have 4 possible answers association with them. I want to create and check the response directly after the user clicks on it. To do this I created this bit of logic here.

<%= @question.prompt %>

<% @question.answers.each do |a| %>
    <%= form_for @response, url: send_wager_response_path, remote: true do |f| %>
        <%= f.hidden_field :user_id, value: current_user.id %>
        <%= f.hidden_field :question_id, value: @question.id %>
        <%= f.hidden_field :answer_id, value: a.id %>
        <%= f.submit "#{a.answer.titleize}", class: 'btn btn-block btn-lg btn-primary' %>
    <% end %>
<% end %>

The problem I have with this is that I am creating 4 different forms on the question page and passing the current user id, the answer id they select, and the question id. This seems a bit not-so DRY. Is there a way to improve on this concept?

I am currently looking into a action-cable based approach since this is a head-to-head application but this is a good place to start.

Why do you submit the current user id in the form? I assume the user is logged in by the statement current_user.id in the form. If you're placing the current user id into the form, you allow the user to change the value (assuming you use the submitted user id in your handler). I know the field is hidden, but everything that an client submits can be manipulated. The only thing to do is open up the developer tools and alter the HTML source. Instead of submitting the id through the form I recommend simply using current_user in the handler and don't submit the user id at all.

You mention you're looking into Action Cable at the moment. However Action Cable is based on the web-socket technology. This could be used, but the main reason they where invented is to keep the connection between the server and client open so the server can send data to the client without the client making a request first. This is mostly done when something changed on the server that has impact on the current view of the client. The term for this would be push notifications or messages.

Since the view doesn't have to change when something changed on the server there is no need to make things more complicated. Instead you want to update the responses when user selects an answer (user initiated). The solution is to update the user response on change (using JavaScript). Rails has a build in system that does this documented in the Ruby on Rails Guides - Working with JavaScript in Rails: 3.2 Customize remote elements.

Let me provide you with an (untested) example:

First we ensure that the correct routes are present so we can update the response in a reasonable fashion.

# config/routes.rb
# ...
resources :questions, only: [] do
  member do
    post 'answer', to: 'responses#answer'
  end
end
# ...

This should create the route /questions/:id/answer that will be handled in ResponsesController#answer. This route should create a helper named answer_question that accepts an question or question id as its first argument.

The controller might look something like this:

# app/controllers/responses_controller.rb
# ...
def answer
  question = Question.find(params[:id])
  answer   = question.answers.find(params[:answer_id])
  response = question.responses.find_or_initialize_by(user: current_user)

  if response.update(answer: answer)
    head :ok
  else
    render 'answer_error'
    # renders /app/views/responses/answer_error.js.erb
    # Here you can write some JS to update the DOM and sets an error
    # message somewhere.
  end
end
# ...

And now for the final part of the puzzle we need the view.

<% # I don't know which view this is, but it replaces the one that OP provided. %>

<%= @question.prompt %>

<% @question.answers.each do |question| %>
  <%= form_for @response, url: answer_question_path(@question), method: :post, remote: true do |f| %>
    <%= f.hidden_field :answer_id, value: answer.id %>
    <%= f.submit a.answer.titleize, class: 'btn btn-block btn-lg btn-primary' %>
  <% end %>
<% end %>

By having an nested route you eliminate the need to submit the question id. And the user id can be left out for reasons mentioned before.

From the tone in your post I assume you also want to drop those forms. This is possible by using the "Customize remote elements" that I linked above. This would leave you with a view that might look something like this.

<%= @question.prompt %>

<% @question.answers.each do |answer| %>
  <%= radio_button_tag "question_#{question.id}_answer", answer.id, false, data: {
    remote: true,
    method: :post,
    url:    answer_question_path(@question),
    params: { answer_id: answer.id }
  } %>
<% end %>

The above would simply generate radio buttons (using radio_button_tag) that are linked to each other (per question). On change a request will be send to the specified URL. This eliminates the need for a form altogether. Keep in mind that no labels are currently present for the radio buttons, you'll have to add them yourself.

Of course you could also use other input HTML input elements to achieve the same result. If you want to stick to the button approach I recommend taking a look at button_tag in combination with the technique shown above. Or button_to which is an helper that generates the wrapping form for you.

As I said, this is an untested example. Might any errors come up let me know in the comments.


Parsing an email string My program needs to parse an e-mail string. There are two possibilities to enter an e-mail-address. Either with the alias or without one, plain the e-mail-address.

1st possibity:

string addressWithAlias = "test my address <bla@blub.com>";

2nd possibility:

string addressWithoutAlias = "bla@blub.com";

So, I wrote two functions:

private static string[] getAddressPartsRegex(string address)
{
    string plainaddress = address.Trim();

    Regex reg = new Regex(@"(.+?(?=<))<(.*@.*?)>");
    var gr = reg.Match(plainaddress).Groups;

    return gr.Count == 1
        ? new[] { plainaddress }
        : new[] { gr[1].Value.Trim(), gr[2].Value.Trim() };
}

private static string[] getAddressParts(string address)
{
    var splittedAdress = address.Split(' ');
    return splittedAdress.Last().Trim().StartsWith("<")
        ? new[] { string.Join(" ", splittedAdress.Take(splittedAdress.Length - 1)), splittedAdress.Last().Trim(' ', '<', '>') }
        : splittedAdress;
}

They both work fine and the results are the same. One uses regex, the other uses Split and Join.
What would you suggest to use, and why? What is the more beautiful function?
Are there any bugs I didn't see?

Consider taking advantage of existing features that could provide an additional layer of validation.

mainly System.Net.Mail.MailAddress

Also as mentioned in a comment, no need to be creating the regular expression every time the function is called.

static Regex mailExpression = new Regex(@"(.+?(?=<))<(.*@.*?)>");
private static MailAddress getAddress(string address) {
    if (address == null) throw new ArgumentNullException("address");
    if (string.IsNullOrWhiteSpace(address)) throw new ArgumentException("invalid address", "address");

    var plainaddress = address.Trim();
    var groups = mailExpression.Match(plainaddress).Groups;

    return groups.Count == 1
        ? new MailAddress(plainaddress)
        : new MailAddress(groups[2].Value.Trim(), groups[1].Value.Trim());
}

According to reference source code, internally MailAddress will try to parse the address given to it.

This avoids having to roll your own parser as one already exists out of the box that has been tried, tested and is stable.

private static MailAddress getAddress(string address) {
    if (address == null) throw new ArgumentNullException("address");
    if (string.IsNullOrWhiteSpace(address)) throw new ArgumentException("invalid address", "address");

    address = address.Trim();
    return new MailAddress(address);
}

You have the added advantage of having a strongly typed object model to work with that will provide you with usable properties.

The following Unit Test demonstrates the desired behavior.

[TestClass]
public class EmailParserTest {
    [TestMethod]
    public void Should_Parse_EmailAddress_With_Alias() {
        //Arrange
        var expectedAlias = "test my address";
        var expectedAddress = "bla@blub.com";
        string addressWithAlias = "test my address <bla@blub.com>";

        //Act
        var mailAddressWithAlias = getAddress(addressWithAlias);

        //Assert
        mailAddressWithAlias
            .Should()
            .NotBeNull()
            .And.Match<MailAddress>(_ => _.Address == expectedAddress && _.DisplayName == expectedAlias);

    }

    [TestMethod]
    public void Should_Parse_EmailAddress_Without_Alias() {
        //Arrange
        var addressWithoutAlias = "bla@blub.com";

        //Act
        var mailAddressWithoutAlias = getAddress(addressWithoutAlias);

        //Assert
        mailAddressWithoutAlias
            .Should()
            .NotBeNull()
            .And.Match<MailAddress>(_ => _.Address == addressWithoutAlias && _.DisplayName == string.Empty);
        ;
    }

    private static MailAddress getAddress(string address) {
        if (address == null) throw new ArgumentNullException("address");
        if (string.IsNullOrWhiteSpace(address)) throw new ArgumentException("invalid address", "address");

        address = address.Trim();
        return new MailAddress(address);
    }
}


Indentation for c# functions I am just looking to improve my indentation on my current code. I'd be happy to hear any reviews and improvements.
public class DataAccess
{

    public List<ComplaintModel> GetComplaint(string _OrderNumber)
    {
        throw new NotImplementedException();
    }

    public void InsertComplaint(
        DateTime _Date,
        string _OrderNumber,
        string _CustomerName,
        string _CustomerContactName,
        string _Telephone,
        string _Email,
        string _CustomerReference,
        string _Product,
        string _PackSize,
        string _BatchNumber,
        DateTime _BestBeforeDate,
        string _QuantityInvolved,
        string _Details,
        string _Comments)
    {
        using(MySqlConnection conn = new MySqlConnection(ConnectionString.ConnString))
        {
            List<ComplaintModel> complaint = new List<ComplaintModel>();

            complaint.Add(new ComplaintModel { Date = _Date,
                OrderNumber = _OrderNumber,
                CustomerName = _CustomerName,
                CustomerContactName = _CustomerContactName,
                Telephone = _Telephone,
                Email = _Email,
                CustomerReference = _CustomerReference,
                Product = _Product,
                PackSize = _PackSize,
                BatchNumber = _BatchNumber,
                BestBeforeDate = _BestBeforeDate,
                QuantityInvolved = _QuantityInvolved,
                Details = _Details,
                Comments = _Comments});

            conn.Execute(@"INSERT INTO customer_complaints
                        (date_taken, order_number, customer_name, customer_contact, telephone, email, customer_reference, product, pack_size, batch_lot_number,
                        best_before_date, quantity_involved, details, comments)
                        VALUES (@Date, @OrderNumber, @CustomerName, @CustomerContactName, @Telephone, @Email, @CustomerReference, @Product, @PackSize, @BatchNumber,
                        @BestBeforeDate, @QuantityInvolved, @Details, @Comments)", complaint);
        }
    }
}

There are two places where I would use var:
var conn = new MySqlConnection(ConnectionString.ConnString)
var complaint = new List<ComplaintModel>();

You no longer have to put braces around a using block, saving you one indentation level:
using var conn = new MySqlConnection(ConnectionString.ConnString);

Don't put the first parameter at the end of the line and the rest on a new line. Put the first parameter on a new line as well:
complaint.Add(new ComplaintModel {
    Date = _Date,
    ...

If you start the Query string on a new line, you can move it back one indentation level:
conn.Execute(
    @"INSERT INTO customer_complaints
    ...


N-tier application with BAL and DAL methods I was just asking this question over here. I'm on a project in which I'm failing to see the point of how a previous developer made decisions. I HAVE taken over this project and wish to make changes.

Same exact method names in DAL and BAL
Static is EVERYWHERE
What should I do with new methods to follow best practices?

Example of existing code:

Calling appliction (could be console app or web app etc. agnostic ):

DataSet DS = CreditMgr.GetCreditRqstInfo(ddlGEO.Text);

BAL

public class CreditMgr
{
   public static DataSet GetCreditRqstInfo(String GeoID)
    {
        try
        {
            DataSet DS = new DataSet();
            DS = CreditIntfDB.GetCreditRqstInfo(GeoID);
            return DS;
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }
}

DAL

public class CreditIntfDB
{
     public static DataSet GetCreditRqstInfo(String GeoID)
    {
        try
        {
            Database DB = new SqlDatabase(Common.ConnectionString);
            String SQLCommand = Common.SPGetRqstInfo;
            DbCommand DBCommand = DB.GetStoredProcCommand(SQLCommand);
            DBCommand.CommandTimeout = Common.CommandTimeOut;
            DB.AddInParameter(DBCommand, "@a_geo_id", DbType.String, GeoID);
            DataSet DS = new DataSet();
            DB.LoadDataSet(DBCommand, DS, new String[] { "CreditRqstInfo" });

            return DS;
        }
        catch (Exception ex)
        {
            throw ex;
        }
    }
}

Yes, the whole point is to have layers of separation, but when the same method names are being used, and static, and each are simply doing the same exact thing with passing in string and returning a DataSet has "code smell" to me.

Any suggestions on better ways?

Let's start with this anti-pattern.

    catch (Exception ex)
    {
        throw ex;
    }

You need to immediately search the code base for every instance of this pattern and remove it immediately. It serves no purpose but to destroy the stack trace. That makes it much harder to debug production issues, because you've no idea where the exception really came from.

(While you're at it, look for any place the previous developer may have been simply swallowing exceptions too.)

Next is this patten:

        DataSet DS = new DataSet();
        DS = CreditIntfDB.GetCreditRqstInfo(GeoID);
        return DS;

This allocated memory, only to instantly overwrite it with the value returned from the static method. This whole thing could be simplified down to a single line.

return CreditInfDB.GetCreditRqstInfo(GeoID);

Now, it's tempting to get rid of the BAL when it's just passing through like this, and eventually you might, but for now, leave it in place. It'll give you some wiggle room while you're replacing that abomination of a data layer with something modern.

Next thing to go is the static declaration on all of the BAL methods. Just remove it (one at a time!), then compile. Follow the errors until you've instantiated instances of the class everywhere you need it. This is called Leaning on the compiler.

Once that's complete, you'll need to stop returning those DataSets from the BAL. This will be by far the most tedious and painful part, but well worth the effort. Start modeling the data and creating POCO's to be returned from the BAL. Databindings client side will need to be updated. This means invasive changes in your views. Use the BAL to map the DataSets coming out of the DAL to your POCOs.

For example:

public class CreditMgr
{
   public IEnumerable<CreditRequestInfo> GetCreditRqstInfo(String GeoID)
    {
        DataSet ds = CreditIntfDB.GetCreditRqstInfo(GeoID);
        return ds.ToCreditRequestInfo();
    }
    //...
}

Where ToCreditRequestInfo() is an extension method that transforms a dataset into a list of your model class.

Now you're in a position to extract an interface for the BAL so it can faked during testing.

Finally, you'll be able to think about ditching the DAL entirely by replacing the guts of the BAL with calls to an ORM like Entity Framework.

public class CreditManager : ICreditManager
{
   public IEnumerable<CreditRequestInfo> GetCreditRqstInfo(String GeoID)
    {
        using (var dbContext = new CreditDbContext())
        {
            return dbContext.GetCreditRequestInfo(GeoID).ToList();
         }
    }

    //...
}

This is not for the faint of heart, and I wouldn't want to tackle it without an excellent refactoring tool like ReSharper and a copy of Working Effectively with Legacy Code by Michael Feathers nearby.

Best of luck. Remember, keep your changes small and safe. Don't change logic if you can avoid it and drive as many tests as you can at the code before changing it. (Although, you'll have to make a few changes to make it testable first... So... Slow and steady friend. Slow and steady.)


String tokenization and replacing a numbered field I created various functions for string manipulation in Lua, where the strings are composed of tokens separated by an ASCII character (which may be the point, comma, semicolon etc.). One of these is the function puttok()

local function tokenize(C, strng)
    local sInput = strng or ""
    local sChar = string.format('%c', C)
    local tReturn = {}
    for sWord in string.gmatch(sInput, "[^"..sChar.."]+") do
        table.insert(tReturn, tonumber(sWord) or sWord)
    end
    return tReturn
end

local function puttok(text,token,N,C)
    local char = string.format("%c", C)
    local n
    local result
    local tokens = tokenize(C, text)
    if (N == 0) or (N > #tokens) then
        result = text
    end
    if (N) then
        n = (N > 0) and N or #tokens + N + 1
        table.remove(tokens,n)
        table.insert(tokens,n,token)
        result = table.concat(tokens,char)
    end
    return result
end

Where

text = string to manipulate
token = token (string) we want insert
N = position in which will be added to the token
C = ASCII code of the token separator

This function is to replace a token with another. Here's an example

local text = "Violets.are.white"
text = puttok(text,"blue",3,46)
print(text)
—› Violets.are.blue

The token "blue" was inserted in position 3 in place of "white," and "Violets" and "are" are respectively in position 1 and 2. The tokens are separated by a dot (.) with ASCII code 46.

Is there a way to improve this code?
Would metatables be usefull in this case?

Don't use the expensive string.format where string.char can do it all.
You should accept a string (of length 1) as separator in tokenize and puttok, and have a default.
Swapping the middle two arguments of puttok allows you to default the replacement to "".
Your second conditional is useless, as it will always be taken: The first one results in a runtime-error if N is not a number.
I wonder why you don't use an early return there anyway?
First removing an element and then inserting a different one at the same index is really inefficient, resulting in a need to move all higher-numbered elements first down and then back.
Just replace it already:

tokens[n] = token

Try for better names, so you don't need to explain so much in comments:

tokenize(C, strng) => explode(delimiter, text)
puttok(text,token,N,C) => replace_token(text, which, replacement, delimiter)
-- Switched the middle two parameters of puttok/replace_token


Mutual Exclusion with Mutexes After I wrote this question: Producer/Consumer programs, I realized that I had forgotten to write a version using mutexes.  I have now solved the Producer/Consumer problem using mutexes and implementing the tips I received from answers on that question.  It appears to be working correctly; however, unlike my other programs, which would run for a while or until the buffer was full, this version tends to interlock the producer/consumer threads, so each character is consumed immediately after it is produced.  Is this because the OS tends to preempt the thread when it releases the mutex if there is another waiting for it?

class Program
{
    const int BuffSize = 10;
    const int ValuesToProduce = 95;

    static void Main(string[] args)
    {
        var producerConsumer = new MutexProducerConsumer(BuffSize, ValuesToProduce);
        Thread p = new Thread(new ThreadStart(producerConsumer.Produce));
        Thread c = new Thread(new ThreadStart(producerConsumer.Consume));
        p.Start();
        c.Start();
    }
}

This is MutexProducerConsumer:

public class MutexProducerConsumer : ProducerConsumerBase
{
    public MutexProducerConsumer(int BuffSize, int ValuesToProduce)
        : base(BuffSize, ValuesToProduce) { }

    volatile int Available = 0;
    Mutex BufferLock = new Mutex(false);
    Mutex IsFull = new Mutex(true);
    Mutex IsEmpty = new Mutex(true);

    public override void Produce()
    {
        for (int i = 0; i < _TotalNumberOfValues; i++)
        {
            while (Available == _BuffSize)
            {
                Console.WriteLine("Wait Producer:");
                IsFull.WaitOne(1000);
            }

            BufferLock.WaitOne();
            _Buffer[i % _BuffSize] = (char)(32 + i % 95);
            Available++;
            Console.WriteLine("Produced: {0}", _Buffer[i % _BuffSize]);
            BufferLock.ReleaseMutex();

            try
            {
                IsEmpty.ReleaseMutex();
            }
            catch
            {

            }
        }
    }

    public override void Consume()
    {
        for (int i = 0; i < _TotalNumberOfValues; i++)
        {
            while (Available < 1)
            {
                Console.WriteLine("Wait Consumer:");
                IsEmpty.WaitOne(1000);
            }

            BufferLock.WaitOne();
            char c = _Buffer[i % _BuffSize];
            Available--;
            Console.WriteLine("Consumed: {0}", _Buffer[i % _BuffSize]);
            BufferLock.ReleaseMutex();

            try
            {
                IsFull.ReleaseMutex();
            }
            catch
            {

            }
        }
    }
}

And this is ProducerConsumerBase:

public abstract class ProducerConsumerBase
{
    protected char[] _Buffer;
    protected int _TotalNumberOfValues;
    protected int _BuffSize;

    protected ProducerConsumerBase(int buffSize, int totalNumberOfValues)
    {
        _BuffSize = buffSize;
        _Buffer = new char[buffSize];
        _TotalNumberOfValues = totalNumberOfValues;
    }

    public abstract void Produce();
    public abstract void Consume();
}

I'll answer your primary question first -

Why is it producing 1 char and then immediately consuming that char?

Look at the producing code carefully:

BufferLock.WaitOne();
_Buffer[i % _BuffSize] = (char)(32 + i % 95);
Available++;
Console.WriteLine("Produced: {0}", _Buffer[i % _BuffSize]);
BufferLock.ReleaseMutex();
IsEmpty.ReleaseMutex(); // Try catch removed, but throws quite often

What you're saying is:

Get exclusive access to the buffer (slow)
Add a character to the buffer
Increment a shared field (in a potentially dangerous way)
Release the buffer (slow)
Potentially throw and catch an exception

At the same time your consumer is doing the analogous:

Get exclusive access to the buffer (slow)
Get a character from the buffer
Decrement a shared field (in a potentially dangerous way)
Release the buffer (slow)
Potentially throw and catch an exception

If you start that work at the same time, you're going to end up in a situation where the consumer is first waiting for the producer to release the buffer, then the producer will be waiting for the consumer to release it on every iteration. The threads are basically playing tennis with the Mutex - back and forth, back and forth. Don't forget that acquiring and releasing a Mutex takes a pretty long time - a couple of microseconds - which might be contributing to the 1 produced, 1 consumed pattern as I think the code will be spending most of its time waiting for the mutex operations.

Notes on the implementation

Most importantly (and I think you know this) a Mutex is utterly overkill for inter thread synchronisation.

A Mutex can only be released by the thread that acquired it. So, the consuming thread will never be able to release the IsFull mutex because it will only ever be acquired by the producing thread. The same is true for the IsEmpty mutex - the producing thread will never be able to release it. I'm sure that you noticed the exception but rather than trying to figure out why it was happening, you've caught and ignored the exception. Having a reason to ignore all exceptions from an operation is quite rare - seeing an empty catch with no comment is usually a sign that something's not right.

Locks/mutexes etc. are expensive - minimize where possible and always hold the lock for the least amount of time possible.

Mutex implements IDisposable, as your MutexProducerProducerConsumer class holds references to them at the class level it should also implement IDisposable.

Your waits are just spin blocking. IsEmpty.WaitOne(1000) will wait up to one second to acquire the mutex.

while (Available < 1)
{
    Console.WriteLine("Wait Consumer:");
    IsEmpty.WaitOne(1000);
}

Your consumer will acquire the mutex and never release it. So your code is basically:

while (Available < 1)
{
    Console.WriteLine("Wait Consumer:");
}

Producer/consumer is more easily implemented with a queue of work. It's weird that your consumer has to know how many things the producer will create.

Some general style points:

This was mentioned in your previous question. I don't think you can actually fall foul of this here, but it's worth remembering that ++ and -- aren't atomic operations.

If the type of the RHS of an assignment is obvious, use var.

You don't need to create an explicit delegate here either:

Thread p = new Thread(new ThreadStart(producerConsumer.Produce));

can be more concisely written as:

var p = new Thread(producerConsumer.Produce);


Random Walk simulator seems slow, are there ways to optimize my code? I'm trying to write a short random walk simulator in which an agent has a 50% chance on each "round" to either move toward x or back to its initial position. If the agent is already at its initial position and has to move back, it stays there instead.

I'm getting a lot of variance in the results and running the function with a distance to x of 2000 steps is already taking a while.

I was thinking I'd run the simulator over 200 steps 100 times and take the average of that, but with 40200 steps when x is 200 steps away on average, it's taking almost 10 seconds to run 100 times, which seems extremely slow.

Are there things I could optimize in my code to make it faster? Also, if you see glaring problems with design, suboptimal memory usage, etc...please don't hesitate to comment on those as well, thank you.

PS: I use a random int function of [0,1] for my 50% probability.

import numpy as np
import random as rand

def randomWalk(x):

    position = 0
    steps = 0

    while (position != x):
        move = rand.randint(0,1)
        steps += 1

        if (move == 1):
            position += 1

        elif (move == 0) & (position != 0):
            position -= 1

    return steps

trials = [0] * 100

for i in range(100):
    trials[i] = randomWalk(200)

sum = 0.

for trial in trials:
    sum += trial

avg = 0.

avg = sum / len(trials)

print avg

trials = [0] * 100
for i in range(100):
    trials[i] = randomWalk(200)

You don't need to create a list of zeroes in order to create a list of random walks.  Just use a list comprehension:

trials = [randomWalk(200) for _ in xrange(100)] # In Python3, you should use range().

sum = 0.
for trial in trials:
    sum += trial
avg = 0.
avg = sum / len(trials)

You shouldn't use sum as a variable name because it conflicts with the built-in function.  You know what?  You can use that built-in function:

avg = sum(trials, 0.) / len(trials)

print avg

I'm assuming that you are using Python2 just because of that line (and because of the decimal points in your numbers).  If you put parentheses around avg, your program would run in both versions.  Since I used xrange() above which is not around in Python3, you can't now; but you could also use range() up there to make it bothways-compatible.  range() takes up more memory than xrange(), in Python2, but with only a hundred numbers, it shouldn't really make much difference anyway.


Div in unordered list - good practice? I want to make a form which looks like this:

     Column A      Column B
1   Textbox AA    Textbox BB
2   Textbox AA    Textbox BB

I've coded it like this:

             <form id="deps" name="deps" action="" method="POST">

                    &nbsp;

&nbsp;

A

B

 1

<input type="text" name="bdmm" id="bdmm" /> M

<input type="text" name="bdinch" id="bdinch" /> I

 2

<input type="text" name="sdmm" id="sdmm" /> M

<input type="text" name="sdinch" id="sdinch" /> I

TW

<input type="text" name="twkg" id="twkg" /> KG

<input type="text" name="twlbs" id="twlbs" /> LBS

                    <input type="submit" value="Calculate" onclick="getText();" />
              </form>

CSS:

form{
    padding:3px 0;
    font-family: Georgia, arial, serif;
}

ul {
    list-style: none;
}

ul li {
    clear:both;
    margin:0px;
    padding:2px 0px;
    vertical-align: middle;
}

input{
    font-size: 1em;
    margin: 5px;
    padding: 5px 8px;
    border: solid 1px #E5E5E5;
    outline: 0;
    width: 100px;
    }

input:focus {
    border: solid 1px #8CDED7;
    }

label {
    margin: 2px;
    padding: 2px;
    text-align: right;
    font-size: 1em;
    text-shadow: 0px 1px 0px #e5e5ee;
    display: inline-block;

    }

label, input, h4 {
    letter-spacing: 1.5px;
    line-height: 20px;
    font-family: Georgia, 'Times New Roman';
    text-transform: uppercase;
}

.field {
    float: left;
    display:inline-block;
    width:  200px;
    vertical-align: middle;
}
.head {
    padding-top: 5px;
}

.er {
    font-size: 1em;
    margin-right: 10px;
    padding: 8px;
    background: #CC0000;
    color: #F7F7F7;
    float:right;
}

.cv {
    float: left;
}
input[type=submit]{
    margin-top: 35px;
    padding: 9px;
    width: auto;
    text-align: center;
    cursor:pointer;
    font-family:  Georgia, arial, serif;
    font-size: 1em;
    color: #565463;
    background: #8CDED7;
    border: 2px solid #8CDED7;
    text-shadow: 0px 1px 0px #e5e5ee;
}

This works fine as I want: http://jsfiddle.net/4kWdQ/. But I would like to know, if putting a div in an unordered list is good practice or not? And anyway I can improve the CSS code?

Divs of any kind are a bit of a "lesser evil" -- they carry no semantic info, and should be used only when (1) you need an element there and (2) there's no semantic equivalent.

In this case, you do have a semantic equivalent.  You have rows, columns, headers...data that must be arranged in those rows/columns and have those headers or it doesn't make sense...what you have here is a bona fide table.  And it should be coded as one, rather than as a list of divs.  ("Tables bad, CSS good" applies to layout tables, not tables in general.  If you have tabular data, putting it in a table just makes sense.)


Finding greatest value in array smaller than x Code review requested to make this code simpler, cleaner, and better. Input array is sorted.

This program finds the greatest number smaller than x.  So, in an array [10 20 30 40] and x = 25, the output should be 20.

public class GreatestValueLesserThanEqualToX {

    public static Integer findGreatestValueLesserThanOrEqualToX (int[] arr, int x) {
        if (arr == null) {
            throw new NullPointerException(" The input array is null. ");
        }
        if (arr.length >= 1) {
            return findGreatestValueLesserThanOrEqualToX (arr, x, 0, arr.length - 1);
        } else {
            return null;  // note the difference wrt greatest value lesser than X.
        }
    }

    private static Integer findGreatestValueLesserThanOrEqualToX (int[] arr, int x, int lb, int ub) {

        assert arr != null;

        final int mid = (lb + ub) / 2;

        /**
         *  Testing boundaries.
         */
        // testing lower boundary.
        if (mid == 0) {
            if (arr[mid] > x) {
                return null;
            }

            // single element array with value lesser than input value.
            if (arr.length == 1) {
                return arr[0];
            }
        }

        //testing higher boundary.
        if (lb == (arr.length - 1) && arr[mid] < x) {
            return arr[mid];
        }

        /**
         *  Testing equalities and duplicates.
         */
        // testing equality and duplicates. eg: consider input like: 1, 2, 2, 5
        if (arr[mid] == x || arr[mid + 1] == x) {
            return x;
        }

        /**
         *  Testing when element is in the range of array elements.
         */
        // input x in range of array elements.
        if (arr[mid] < x && arr[mid + 1] > x) {
             return arr[mid]; // note the difference wrt greatest value lesser than X.
         }

         if (arr[mid] < x) {
             return findGreatestValueLesserThanOrEqualToX (arr, x, mid + 1, ub);
         } else {
             return findGreatestValueLesserThanOrEqualToX (arr, x, lb, mid);
         }
     }
}

Interface Design

The class could be better named.  (As @DaveJarvis points out, it's more of a namespace than a class, but I'm OK with a design that is not pure OOP.)  I suggest BinarySearcher: it describes what it can do, without repeating the name of the function, and still leaves it open for you to add a findSmallestValueGreaterThanOrEqualToX() function later.

Returning an Integer is weird, especially since the inputs are unboxed ints.  I know, you want to be able to return null if no suitable element is found.  I have another suggestion: return the index of the element found rather than the element itself; if no suitable element exists then return -1.  That behaviour is reminiscent of String.indexOf(), which all Java programmers should be familiar with.  Knowing the index, the caller can easily retrieve the element from the array.

We could also shorten the function name a bit.  I suggest that the method signature should be

/**
 * Given a sorted array and a limit, finds the rightmost element
 * whose value does not exceed the limit.
 *
 * @param data An array sorted in ascending order
 * @param limit The maximum value to look for
 * @return The index of the last element in data whose value is less
 *         than or equal to limit.  If no such element exists, returns -1.
 */
public static int greatestIndexNotExceeding(int[] data, int limit)

Search Logic

There is no reason for your helper function to consult arr.length.  The helper function's job is to look between indices lb and ub.  The caller will vouch for the validity of those bounds.  What happens beyond that range is none of the helper function's business.

You have seven cases, which is way too many.  You should only need four:

No suitable element
Found the result
Consider the upper half of the range
Consider the lower half of the range

Null-Handling

There's no need to throw new NullPointerException() explicitly.  The very next line will raise a NullPointerException naturally when it tries to access arr.length.  The arr != null assertion is similarly pointless.

When might it be beneficial to check for null explicitly?  When a constructor or a setter method accepts an argument and stores it for future use, but does not try to use it immediately.  In those cases, it can be hard to track down later how an instance or class variable came to be null.

Proposed Solution

public class BinarySearcher {
    /**
     * Insert JavaDoc here…
     */
    public static int greatestIndexNotExceeding(int[] data, int limit) {
        if (data.length < 1) {
            return -1;
        }
        return greatestIndexNotExceeding(data, limit, 0, data.length - 1);
    }

    private static int greatestIndexNotExceeding(int[] data, int limit, int lb, int ub) {
        final int mid = (lb + ub) / 2;

        // Need to go lower but can't
        if (mid == lb && data[mid] > limit) {
            return -1;
        }

        // Found a candidate, and can't go higher
        if (data[mid] <= limit && (mid == ub || data[mid + 1] > limit)) {
            return mid;
        }

        if (data[mid] <= limit) {
            // Consider upper half
            return greatestIndexNotExceeding(data, limit, mid + 1, ub);
        } else {
            // Consider lower half
            return greatestIndexNotExceeding(data, limit, lb, mid);
        }
    }
}

It should be possible to restructure the code to reduce the number of comparisons between data[mid] and limit, but I think it's more readable this way.


Establishing parameters for a sales report, with default boolean and date values The idea is that I am getting the params from a GET request in the form of a Hash (symbol→str) and I want to transform them into dates and booleans. For that reason, I created two private methods to_date and to_b.

class Report
  include ActiveModel::Model

  attr_accessor :start_date, :end_date, :show_sales, :show_daily_active_users,
                :show_registered_cc, :show_all_time_registered_cc, :show_users

  def initialize(params)
    params ||= {}
    @start_date = to_date(params, :start_date, Time.zone.today - 7.days)
    @end_date = to_date params, :end_date, Time.zone.today
    @show_sales = to_b params, :show_sales, true
    @show_daily_active_users = to_b params, :show_daily_active_users, true
    @show_registered_cc = to_b params, :show_registered_cc
    @show_all_time_registered_cc = to_b params, :show_all_time_registered_cc
    @show_users = to_b params, :show_users
  end

  def sales
     (...)
  end

  (...)

  private

  def to_date(params, key, default_val)
    if params.key?(key)
      Date.parse(params[key])
    else
      default_val
    end
  end

  def to_b(params, key, default_true = false)
    default_val = '1' if default_true
    params.fetch(key, default_val) == '1'
  end
end

The problem is that Rubocop says that the Assignment Branch Condition size for the method initialize is too high (15.81 over a limit of 15).

Is there anything I could change to fix that? I don't think that creating a new class just to do that is worth it.

You could reduce branching and improve readability by requiring the default for to_b to be explicitly stated.  (It seems a bit excessive to have a default for the default, don't you think?)

The branching is not my primary concern, though.  The constructor seems to have a very copy-and-paste pattern.  Metaprogramming could help here, I think.  It would be nice to write…

class Report
  include ActiveModel::Model
  include ParamAccessors

  date_param_accessor(:start_date) { |today| today - 7.days }
  date_param_accessor :end_date
  bool_param_accessor :show_sales, true
  bool_param_accessor :show_daily_active_users, false
  bool_param_accessor :show_registered_cc, false
  bool_param_accessor :show_all_time_registered_cc, false
  bool_param_accessor :show_users, false

  def initialize(params)
    @params = params
  end

  def sales
     …
  end
end

This mixin could make that possible:

module ParamAccessors
  def self.included(base)
    base.extend ClassMethods
  end

  module ClassMethods
    def date_param_accessor(name, default=nil)
      attr_writer(name)
      define_method(name) do
        if instance_variable_defined?("@#{name}")
          instance_variable_get("@#{name}")
        else
          @params.fetch(name, default) or
          (yield Time.zone.today if block_given?) or
          return Time.zone.today
        end
      end
    end

    def bool_param_accessor(name, default)
      attr_writer(name)
      define_method(name) do
        if instance_variable_defined?("@#{name}")
          instance_variable_get("@#{name}")
        else
          @params.fetch(name, default)
        end
      end
    end
  end
end

This assumes that the object is transient, such that the constructor and the start_date/end_date methods will be called on the same day.


Faster binary search to find the index in Python? I am implementing a binary search algorithm and my code is as shown bellow:
def binary_search(a, x):
    left, right = 0, len(a)
    if right == 0: return -1
    if x < a[0] or x > a[-1]:
        return -1
    elif x == a[0]:
        return 0
    elif x == a[-1]:
        return right - 1
    mid = right // 2
    a_mid = a[mid]
    if x == a_mid:
        return mid
    elif x < a_mid:
        return binary_search(a[left:mid], x)
    elif x > a_mid:
        idx = binary_search(a[mid+1:right], x)
        if idx == -1:
            return -1
        else:
            return mid + 1 + idx

I tested it and cannot find any way to improve it by complexity. Someone suggested that I may "have used an incorrect base condition to terminate your loop" but I still cannot figure out where the problem lies.
Could anyone please help? Thanks in advance.

I can see the problem with your code. Why This "mid = right // 2  " ?
You can try below code
def binarySearch(arr, low, high, key):
    if high >= low:
        mid = (high + low) // 2
        if arr[mid] == key:
            return mid
        elif arr[mid] > key:
            return binarySearch(arr, low, mid - 1, key)
        else:
            return binarySearch(arr, mid + 1, high, key)
    else:
        return -1


Best way to handle the error in async node To catch errors, I have written if-else blocks in every function which looks bad. Please suggest a better way to handle errors in async node.

   async.waterfall([

            function(callback){

                fnOne.GetOne(req, res,function(err,result) {

                  if(err){
                    console.error("Controller : fnOne",err);
                    callback(err,null);
                  }
                  else{

                    var fnOne = result;

                   callback(null, fnOne);
                  }
                })
            },

            function(fnOne, callback){

               fnTwo.two(fnOne,function(err,result) {
                  if(err) {
                    console.error(err);
                    callback(err,null);
                  }
                  else{

                    callback(null, context);
                  }
               })
            }
        ], function (err, result) {

           if(err){
              console.error("Controller waterfall Error" , err);
              res.send("Error in serving request.");
           }
        });

Check out promises. They are not (yet) available natively in Node.js, but you can use Q library.

Promises allow you to make your control flow explicit with methods like catch and finally instead of hiding it in callbacks.

Also you won't need async with promises.

Q provides a convenience method to “convert” a function that accepts a callback to a promise-returning function.

I don't understand what fnOne or fnTwo is your code (seemingly objects and not functions), so I can't really translate your example, but it would look similar to this:

var Q = require('q');

// Convert our functions to promise-returning functions
var getOne = Q.nbind(fnOne.GetOne, fnOne),
    getTwo = Q.nbind(fnTwo.two, fnTwo) ;

getOne(req, res)
  .then(function (result) {
    return getTwo(result);
  })
  .then(function (result) {
    // do something useful with final result
  })
  .catch(function (err) {
    console.error("Three was an error", err);
    res.send("Error in serving request.");
  })
  .done();


XCTestCase#waitFalseExpectationUntilTimeout implementation I am working on an iOS project and I'm in charge of testing most parts of it. As I write some tests, I often have the need to wait for an asynchronous method to finish, and then test that something did not happen.

For example, try to login with an invalid username and password, wait until the network communications are finished, then test that the user was not able to login

Swift has some nifty methods for testing these sort of things; the ones I use the most are expectationForPredicate and waitForExpectationsWithTimeout. These methods are good for testing that something happened after the asynchronous method finishes. However, to test that something did not happen (which is what I want), there doesn't seem to be anything designed specifically for this.

I can't use predicates because that will cause the test to timeout early
I cannot modify the code being tested because it is an app lifecycle callback and we testers generally do not modify any code, we just test

Which is why I designed this method

extension XCTestCase {
    func waitFalseExpectationsUntilTimeout(timeout: NSTimeInterval, expectations: [XCTestExpectation], handler: XCWaitCompletionHandler?) {

        class CompleteSelector {
            @objc func fullFillExpectations(timer: NSTimer) {
                let expectations = timer.userInfo as! [XCTestExpectation]
                for expc in expectations {
                    expc.fulfill()
                }
            }
        }

        let smaller = timeout < 0 ? 0 : timeout.advancedBy(-0.5)

        NSTimer.scheduledTimerWithTimeInterval(smaller, target: CompleteSelector(), selector:
            Selector("fullFillExpectations:"), userInfo: expectations, repeats: false)

        waitForExpectationsWithTimeout(timeout, handler: handler)
    }
}

The main thing the function does is to schedule a timer and fulfill all expectations just before the waitForExpectationsWithTimeout method times out. Is this the best way to deal with this situation? What can I improve with this method (apart from commenting)? Any concurrency issues I should be worried about?

To add some context, waitForExpectationsWithTimeout will cause the test to fail if non of the expectations are fulfilled before it timesout, which is why I had to fulfill all expectations just before this timeout occurs

This is an extremely overcomplicated solution to a relatively straightforward problem.

The waitForExpectationsWithTimeout method should effectively only be used assure that a particular asynchronous expectation was fulfilled.

So, in the case where we are testing whether or not the user was successfully logged in, it'd be tempting to write something along the lines of:

func testLogin() {
    let loginExpectation = expectationWithDescription("User successfully logged in")

    LoginManager.doLogin { success in
        if success {
            loginExpectation.fulfill()
        }
    }

    waitForExpectationsWithTimeout(10, handler: nil)
}

But problematically, this test can fail for two reasons:

The user wasn't successfully logged in.
The expectation wasn't fulfilled in the time limit.

If the test fails, we can't be sure whether it would have passed given a longer time limit or if it returned very quickly but wasn't successful.

So, we need to modify our test to look more like this:

func testLogin() {
    let networkExpectation = expectationWithDescription("Network request completed")

    LoginManager.doLogin { success in
        XCTAssertTrue(success)
        networkExpectation.fulfill()
    }

    waitForExpectationsWithTimeout(10, handler: nil)
}

Now, the test determines whether or not the asynchronous code executed, and if so, we verified whether or not the login was a success (or whatever the success variable represents here).

Now, you're looking for the opposite case.  You want to know not whether or not the user logged in, but whether or not the user was prevented from logging in.  And that's as simple as inverting an XCTAssertTrue to an XCTAssertFalse.

func testLogin() {
    let networkExpectation = expectationWithDescription("Network request completed")

    LoginManager.doLogin { success in
        XCTAssertFalse(success)
        networkExpectation.fulfill()
    }

    waitForExpectationsWithTimeout(10, handler: nil)
}

Importantly, this pattern doesn't require any complicated code (that we would have to of course write tests around ourselves).  Our tests are using stuff purely out of the Xcode test suite box--exactly what any one looking at our code base would expect to see.  It's very straight forward what is going on here.

And we are able to distinguish between whether the asynchronous call back ever happened or if it did happen but we got a bad result.  There's a huge difference here between a test failing because the asynchronous code didn't run before our time out and it did run but it gave an incorrect result.  We need to clearly distinguish between these results, and doing so doesn't require writing any complicated code whatsoever.

An important note here, make sure that fulfilling the expectation is the very last thing you do in the asynchronous callbacks, or you may set up a race condition.

And finally, your tests should certainly be far more robust than this.  Ultimately, we should be testing with mocked network responses and we should know, for each test, exactly what network response we will get.  We should test that whatever code is doing the login returns back the correct information to us.  There's a difference between the server allowing the user to log in successfully and denying the user with invalid credentials.  There's also a difference between invalid credentials and getting any number of various server errors (most commonly, errors in the 5xx range).  Getting a 503 and getting an invalid credential response are far from the same thing, and your tests should validate you're getting what you expect to get.

So, I've basically just told you to completely scrap all the code you've written.  And I'll stand by that.  There's no reason for the code you've written to exist.  But, we can still talk about some of the issues your existing code does have.

This function name has a weird name: waitFalseExpectationsUntilTimeout.  It's hard for me to tell by name alone what to expect this method to do.  It looks similar to the existing waitForExpectationsWithTimeout(timeout: NSTimeInterval, handler: XCWaitCompletionHandler?) method, but it has this weird array of expectations I'm supposed to pass in.  The name needs to be better.  Something like assertUnfulfilledExpectationsWithTimeout.

Even with the better name, we're in extraordinary trouble here.  As far as I can see, there's no public means of checking whether or not an expectation has been previously fulfilled.  Your code doesn't really do anything beyond fulfilling all of the expectations in the last moment before the test completes.  It doesn't assure that they weren't fulfilled earlier.

let smaller = timeout < 0 ? 0 : timeout.advancedBy(-0.5)

This variable deserves a better name, and this logic either deserves a comment or it should be eliminated altogether.  I'd opt for eliminating it altogether and throwing an exception if the user has passed a value less than 0.5.  And finally, this logic is ultimately problematic, because you can end up with negative time (if timeout is less than 0.5).

You should probably opt for some logic that looks more like this:

if timeout < 0.5 {
    // fulfill expectation immediately
}
else {
    // set up timer with timeout - 0.5 to fulfill expectations
}

Finally, there are several issues with the nested class:

class CompleteSelector {
    @objc func fullFillExpectations(timer: NSTimer) {
        let expectations = timer.userInfo as! [XCTestExpectation]
        for expc in expectations {
            expc.fulfill()
        }
    }
}

First, we don't need to mark this as an @objc function.  We can instead mark it as dynamic.

Second, we're force-casting, but we're not Jedi.  This would not pass my personal set of swiftlint rules where force-casting is considered an error and refuses to compile.

Third, expc is a meaningless variable name, and there's no real value in abbreviating here.

Fourth, you've misspelled fulfill even though it's spelled correctly several other times in this context by virtue of autocompleting method names.

Fifth, why don't we just add a method to the extension to handle this instead of over complicating with a nested class with a weird name?

enum ExpectationArgumentError: ErrorType {
    case InvalidTimeInterval, NoExpectations
}

extension XCTestCase {
    func assertUnfulfilledExpectations(timeout: NSTimeInterval, expectations: [XCTestExpectation], handler: XCWaitCompletionHandler?) throws {
        if expectations.empty {
            throw ExpectationArgumentError.NoExpectations
        }

        let fulfillTime = timeout - 0.5
        if fulfillTime < 0 {
            throw ExpectationArgumentError.InvalidTimeInterval
        }

        NSTimer.scheduledTimerWithTimeInterval(fulfillTime, target: self, selector: "fulfillExpectations:", userInfo: expectations, repeats: false)

        waitForExpectationsWithTimeout(timeout, handler: handler)
    }

    private dynamic func fulfillExpectations(timer: NSTimer) {
        if let expectations = timer.userInfo as? [XCTestExpectation] {
            for expectation in expectations {
                expectation.fulfill
            }
        }
    }
}

So, that's a significantly cleaned up and safer version of your code... but again, it doesn't actually assert that these expectations were not fulfilled before you fulfilled them (as far as I know that's impossible).  And ultimately, it's a quite a lot of code to implement, test, maintain, and verify that it works... and that is especially so when we compare that to the simply expectation+assertion combo I outlined at the top of this answer.

I wanted to add an addendum in case the issue was that there is a particular block of code that is only executed in the case of a successful login.  If this is the scenario, there are only two possibilities.

The first is that there are separate success and failure blocks.  Something along these lines:

LoginManager.doLogin("username", password: "password",
    success: {
        // login was successful
    },
    failure: {
        // login failed
    }
)

In this case, the gist is roughly the same as what I've outlined at the top.  We do want to fulfill the expectation in both blocks, but in the success block, we'll just add an XCTFail("User was allowed to log in") so that we still wait around for this block (or the failure block) to execute, but if it takes this path, we fail for the specific reason that the wrong branch was entered (as opposed to simply taking the right path but not taking it quickly enough).

The other case is that there is only a callback for the success path:

LoginManager.doLogin {
    // if logged in, this is called... otherwise, nothing
}

If THIS is the design, I don't have a clue how your app manages to give any feedback to the user at all that something went wrong during the login process... I know you can't modify the code, but you need to be telling the people who can that they need to fix this problem.


Find how many inequalities can be simultaneously satisfied Inequality
Given n inequalities about X, how many of them can hold at the same time at most?
The inequalities are in the following forms:

X < C

X <= C

X = C

X > C

X >= C

input

First line with an integer n.

Following n lines, each line with an inequality.

Data Limit:
1 ≤ N ≤ 50, 0 ≤ C ≤ 1000

output

One line with an integer, denoting the answer.

Sample Input
4
X = 1
X = 2
X = 3
X > 0

Sample Output
2

Here is my code:
#include <iostream>
#include <string>

#include <vector>

#include <map>
#include <set>

using namespace std;

int main() {
    //cout<<"请输入:"<<endl;
    int T;
    cin >> T;
    int count = T;

    vector<float> arrayOne(T, 0);
    vector<int> arrayTwo(T, 0);
    map<string, int> symbols = {
        pair<string, int>(">", 1),
        pair<string, int>(">=", 2),
        pair<string, int>("=", 3),
        pair<string, int>("<=", 4),
        pair<string, int>("<", 5)
    };

    while(T--)
    {
        string sOne;
        cin >> sOne;
        string sThree;
        cin >> sThree;
        arrayTwo[count-T-1] = symbols.find(sThree)->second;
        std::string sTwo;
        std::cin >> sTwo;
        float num = stof(sTwo);
        arrayOne[count-T-1] = num;
    }
    set<float> answerSet;
    for (float numPiece: arrayOne){
        answerSet.insert(numPiece-0.5);
        answerSet.insert(numPiece);
        answerSet.insert(numPiece+0.5);
    }
    int answer = 0;
    for (set<float>::iterator testNum=answerSet.begin(); testNum != answerSet.end(); ++testNum){
        //   cout<<*testNum;
        int passNum = 0;
        int i = 0;
        for (int numTwo: arrayTwo){
            switch (numTwo) {
                case 1:
                {
                    if (*testNum > arrayOne[i]) {
                        passNum += 1;
                    }
                }
                    break;
                case 2:
                {
                    if (*testNum >= arrayOne[i]) {
                        passNum += 1;
                    }
                }
                    break;
                case 3:
                {
                    if (*testNum == arrayOne[i]) {
                        passNum += 1;
                    }
                }
                    break;
                case 4:
                {
                    if (*testNum <= arrayOne[i]) {
                        passNum += 1;
                    }
                }
                    break;
                case 5:
                {
                    if (*testNum < arrayOne[i]) {
                        passNum += 1;
                    }
                }
                    break;
                default:
                    break;
            }
            i++;
        }
        answer = max(answer, passNum);
    }
    cout<<answer<<endl;
    return 0;
}

Very intuitive , just get all the boundary numbers, then find the max passes.
How to make it more efficient?

Avoid using namespace std;

Bringing all names in from a namespace is problematic; namespace std particularly so. See Why is “using namespace std” considered bad practice?.

Don't do everything in main()

Putting all the code into the main() function makes it harder to test parts of the program separately.  If we at least create a function that can read from an arbitrary std::istream, then we're able to do more repeatable testing (by passing suitable std::istringstream objects with our test data in).

Always test that input was successfully read

When we read values from an input stream, we must check that the read was successful before using those values.  A simple way to do that is to set the stream to throw exceptions on failures.

We also need to be more robust when we've read the inequality type, rather than just assuming it will be found in symbols.

Consider using unsigned types

We know that N can't be negative, and the answer is a count of results, which must necessarily be non-negative, too.

Magic numbers

The magic numbers 1..5 used for the different inequalities should be given names.  But there's something better that we can do: we can change their type to be the actual functions:

#include <functional>

///...
{

    using inequality = std::function<bool(float,float)>;

    vector<float> arrayOne(T, 0);
    vector<inequality> arrayTwo(T);
    static const std::map<string, inequality> symbols =
        {
         { ">", std::greater<float>() },
         { ">=", std::greater_equal<float>() },
         { "=", std::equal_to<float>() },
         { "<=", std::less_equal<float>() },
         { "<", std::less<float>() },
        };

Then the big switch becomes much simpler:

    for (auto const& numTwo: arrayTwo){
        passNum += numTwo(*testNum, arrayOne[i]);
        i++;
    }

(I must point out in passing that those names - arrayOne, arrayTwo, numTwo - are really uninformative and unhelpful; you really do need to spend some time thinking of better names if you want to be able to understand your code again later.)

Reconsider the algorithm

Instead of performing all the comparisons on a selection of values (which is O(n²) unless there's many duplicate C values), let's consider what happens to the number of satisfied inequalities as we move X from -∞ to +∞:

each < changes the count by -1 infinitesimally before C
each <= changes the count by -1 infinitesimally after C
>= and > change the count by +1 before or after C, respectively
= changes the count by +1 before and -1 after.

So we can keep an ordered map of those changes:

struct delta
{
    int before;
    int after;

    delta& operator+=(const delta& other)
    {
        before += other.before;
        after  += other.after;
        return *this;
    }
};

Then, just add the appropriate delta values for each inequality:

static const std::map<string, delta> symbol_delta =
    {
     { ">",  {  0,  1 } },
     { ">=", {  1,  0 } },
     { "=",  {  1, -1 } },
     { "<=", {  0, -1 } },
     { "<",  { -1,  0 } },
    };

for (auto i = 0u;  i < lines;  ++i) {
    std::string var;
    std::string symbol;
    double value;

    in >> var >> symbol >> value;
    if (var != "X") {
        throw std::invalid_argument(var);
    }

    auto entry = symbol_delta.find(symbol);
    if (entry == symbol_delta.end()) {
        throw std::invalid_argument(symbol);
    }

    const delta& d = entry->second;
    changes[value] += d;
    if (d.before + d.after < 0) {
        // it's a less-than relation, so true at -∞
        ++count;
    }
}

When that's done, start with a count of the number of inequalities that are true at -∞, (i.e. the total number of < and <= we've seen) and update that count as we walk the map, keeping track of the maximum:

auto max_count = count;
// now, advance to +∞; updating count as we go
for (auto& entry: changes) {
    const delta& d = entry.second;
    count += d.before;
    if (count > max_count) { max_count = count; }
    count += d.after;
    if (count > max_count) { max_count = count; }
}

The complexity of this algorithm is O(n log n), because we're doing a O(log n) insertion into the map n times.

Full working alternative

#include <istream>
#include <map>
#include <string>

struct delta
{
    int before;
    int after;

    delta& operator+=(const delta& other)
    {
        before += other.before;
        after  += other.after;
        return *this;
    }
};

unsigned int max_inequalities(std::istream& in)
{
    // ensure stream '>>' succeeds or throws
    in.exceptions(std::ios_base::badbit | std::ios_base::failbit);

    unsigned int lines;
    in >> lines;

    std::map<double,delta> changes;
    unsigned int count = 0;     // count of satisfied inequalities at -∞

    static const std::map<std::string, delta> deltas =
        {
         { ">",  {  0,  1 } },
         { ">=", {  1,  0 } },
         { "=",  {  1, -1 } },
         { "<=", {  0, -1 } },
         { "<",  { -1,  0 } },
        };

    for (auto i = 0u;  i < lines;  ++i) {
        std::string var_name;
        std::string comparison;
        double value;

        in >> var_name >> comparison >> value;
        if (var_name != "X") {
            throw std::invalid_argument(var_name);
        }

        auto entry = deltas.find(comparison);
        if (entry == deltas.end()) {
            throw std::invalid_argument(comparison);
        }

        const delta& d = entry->second;
        changes[value] += d;
        if (d.before + d.after < 0) {
            // it's a less-than relation, so true at -∞
            ++count;
        }
    }

    auto max_count = count;
    // now, advance to +∞; updating count as we go
    for (auto& entry: changes) {
        const delta& d = entry.second;
        count += d.before;
        if (count > max_count) { max_count = count; }
        count += d.after;
        if (count > max_count) { max_count = count; }
    }

    return max_count;
}

#include <iostream>
#include <sstream>

int main()
{
    auto input = std::istringstream{"4\n"
                                    "X = 1\n"
                                    "X = 2\n"
                                    "X = 3\n"
                                    "X > 0\n"
    };

    std::cout << max_inequalities(input) << '\n';
}


Dirty but Efficient Texture Atlas in Spritekit I have spent quite a bit of time trying to optimize the performance of my 2D mining game.  After a lot of research, experimentation, and testing, I finally found something very interesting that improved my performance pretty dramatically when rendering lots of tiles on the screen.

Initially I divided my textures up into texture atlases based on the way that I had seen many examples of texture organization for animations.  I put all the textures for each tile in one directory with a name like dirtimages.atlas, and then I loaded those textures into the game by iterating over the images in the directory and accounting for the number of formats I provided images for (there are different formats for smaller versus larger resolution devices).  I had a folder for each set of textures, so dirtimages.atlas, rockimages.atlas, etc.  This provided very clear organization in the code, and I liked it very much.

Unfortunately when profiling the number of draws of the scene, I was seeing pretty much one draw for every single tile (hundreds of draws).  I was hoping that the draws would not exceed the number of atlases, but for some reason, splitting the images up into different atlases caused the number of draws to increase by a huge amount.

Here is my workaround for this problem.  I consider it a dirty hack, but I think that this approach is the best one for performance.  I do remember seeing a mention somewhere on the internet about putting all of the images in one texture atlas, but I cannot find that link.  I did find this link on stack overflow about efficient texture atlas setups and it recommends pretty much what I have implemented here.

First I load up ALL of the textures into memory like this:

int numFormats = 3;
_tilesFromAtlas = textureAtlas(@"tileimages", @"tile", numFormats);

NSArray* textureAtlas(NSString *name, NSString *prefix, int numFormats) {
    NSMutableArray *atlasImages = [[NSMutableArray alloc]init];
    SKTextureAtlas *atlas = [SKTextureAtlas atlasNamed:name];
    if (atlas) {
        int numImages = (int)atlas.textureNames.count;
        for (int i = 1; i <= numImages/numFormats; i++) {
            SKTexture *texture = [atlas textureNamed:[NSString stringWithFormat:@"%@%d", prefix, i]];
            [atlasImages addObject:texture];
        }
    }
    return atlasImages;
}

Now when I am assigning a texture to a sprite in the scene, I have to keep track of the number of images that I have for each different tile. Also, the way I have this set up, they have to have the same number of images:

-(SKTexture *) textureForType:(BlockType)type damageLevel:(int)damageLevel {
    int numImagesEach = 4;
    switch (type) {
        case BlockTypeDirt:
        {
            int amount = (type * numImagesEach) + damageLevel;
            return _tilesFromAtlas[amount];
        }
        case BlockTypeRock:
        {
            int amount = (type * numImagesEach) + damageLevel;
            return _tilesFromAtlas[amount];
        }
        default:
            break;
    }
    return nil;
}

I would love to get feedback on this approach.  I think that it suffers from readability problems, but there is no denying the huge performance increase. After making this change the number of draws is usually somewhere between 1 and 2, and sometimes goes up to 10 or so if I zoom out a ways.

Just some quick minor complaints...

If braces are absolutely necessary in a switch, there's no reason why they shouldn't also be Egyptian-style like everywhere else in your code:

case BlockTypeDirt: {
    int amount = (type * numImagesEach) + damageLevel;
    return _tilesFromAtlas[amount];
}

But this seems unnecessary altogether.  We can refactor the switch statement as such:

switch(type) {
    case BlockTypeDirt:
    case BlockTypeRock:
        return _tilesFromAtlas[(type * numImagesEach) + damageLevel];
    default:
        return nil;
}

Now, I'm not sure I actually particularly like cramming all that inside the brackets, and the extra variable doesn't hurt, so we could keep it in the switch, but still, let's combine these cases and fix our brackets:

switch(type) {
    case BlockTypeDirt:
    case BlockTypeRock: {
        int amount = (type * numImagesEach) + damageLevel;
        return _tilesFromAtlas[amount];
    }
    default:
        return nil;
}

Our code has some borderline magic numbers:

int numFormats = 3;

int numImagesEach = 4;

I'll deal with the latter first.  It's defined as the first line of a function, it's only used within the function, and it's used as a constant.

static int const kNumberOfImagesEach = 4;

The other variable is a little hard to say.  It may be that it's simply out of context, and in reality, might be different values, as this number is passed into the function that actually uses it.  I'll let you use your best judgment on that one, but you should think about it.

int numImages = (int)atlas.textureNames.count;

This is an example of why I default to NSInteger rather than int in all of my Objective-C code--that's what Apple has defaulted to, and I'd rather not have to cast all of my returns from Apple's methods, which you'll probably find that on balance, you're using Apple methods more than any other single developer's methods.


My first javascript sorting I started learning JavaScript a week ago, and I made a sorting function on my own. It does work, but I need reviews and how to write a better one.

Art

Mobile

Education

Games

Magazines

Sports

var list = document.getElementById("list");
var myList = list.getElementsByTagName("li");
var a = [];
for (var i = 0; i < myList.length; i++) {
    a[i] = myList[i].innerHTML;
}
a.sort();
for (var i = 0; i < myList.length; i++) {
    myList[i].innerHTML = a[i];
}

Output:

  Art
  Education
  Games
  Magazines
  Mobile
  Sports

Some notes:

var myList: Don't use generic names like myList. My list of what? Variable names are very important, they should inform about the nature of their content, not their type (although it's indeed important to use singular variable names for single elements and plurals for collections).
var myList = list.getElementsByTagName("li");. What if list is null? no checks?
myList[i].innerHTML = a[i];. You are overwriting HTML contents, it would probably be better to move DOM elements instead.

I'll leave a pure Javascript re-write to others, if you are interested in solutions that use external libraries, using jQuery and underscore I'd write:

var sorted_lis = _.sortBy($("#list li"), function(li) { return li.innerText; });
$("#list").empty().append(sorted_lis);

The original ten lines reduced to two (one actually, but it's good to break things a bit and give names to intermediate values), thanks to functional style (underscore) and easy DOM manipulation (jQuery).


Finding maximum subarry sum Looking for feedback on a question I solved in C++. It is a leetcode problem and I used divide and conquer to solve the problem.

  53. Maximum Subarray

  Given an integer array nums, find the contiguous subarray (containing at least one number) which has the largest sum and return its sum.

  Example:

Input: [-2,1,-3,4,-1,2,1,-5,4],
Output: 6
Explanation: [4,-1,2,1] has the largest sum = 6.

  Follow up:

  If you have figured out the \$\mathcal{O}(n)\$ solution, try coding another solution using the divide and conquer approach, which is more subtle.

I am looking for feedback in terms of logic and also the implementation. If I search for a C++ solution online, I only see problem solved using arrays and I have used vector to solve to the problem while utilizing iterators.

Any feedback would be great.

Thanks!

int FindMaximumSubarray(const vector<int> &vec) {
  if (vec.size() == 1) {
    return vec.at(0);
  }
  int midIndex = vec.size() / 2;
  vector<int> leftArray(vec.begin(), vec.begin() + midIndex);
  vector<int> rightArray(vec.begin() + midIndex, vec.end());
  int maximumSumLeftSubarray = FindMaximumSubarray(leftArray);
  int maximumSumRightSubarray = FindMaximumSubarray(rightArray);
  int maximumSumCrossingSubarray = FindMaximumSubarrayCrossing(vec);
  return FindMaximumNumber(maximumSumLeftSubarray,
                           maximumSumRightSubarray,
                           maximumSumCrossingSubarray);
}

int FindMaximumSubarrayCrossing(const vector<int> &vec) {
  int midIndex = vec.size() / 2, leftSum = INT_MIN, rightSum = INT_MIN, sum = 0;
  for (auto itr = vec.rbegin() + midIndex; itr != vec.rend(); ++itr) {
    sum += *itr;
    if (sum > leftSum) leftSum = sum;
  }
  sum = 0;
  for (auto itr = vec.begin() + midIndex + 1; itr != vec.end(); ++itr) {
    sum += *itr;
    if (sum > rightSum) rightSum = sum;
  }
  if (leftSum == INT_MIN || rightSum == INT_MIN) {
    return (leftSum == INT_MIN) ? rightSum : leftSum;
  }
  return (leftSum + rightSum);
}

int FindMaximumNumber(const int &a, const int &b, const int &c) {
  if (a >= b && a >= c) return a;
  if (b >= a && b >= c) return b;
  if (c >= a && c >= b) return c;
}
```

You're creating a bunch of unnecessary vector copies. Try passing iterators into FindMaximumSubarray instead of a vector.

You can find the max of an initializer list of numbers using std::max.

You don't need to pass ints as const refs.

Are you sure this is more performant than a linear solution? What is your reasoning? Can we see your linear version?

Your code looks like it might have potential overflow errors. Maybe that's not important.


PHP New User Creation I'm learning more about PHP, so I've decided to create a simple login/create account system. Entering information for creating a new account sends the data to my localhost machine, using MySQLi and the web server MySQL. I'm looking for feedback about security, efficiency, and overall code. I would like to kick old habits to the curb before it's too late. Any and all help is appreciated and considered. Thank you in advance!

create.html

<!DOCTYPE html>

        <meta charset="UTF-8">
        <title>create.html</title>

        <link rel="stylesheet" type="text/css" href="style.css">

            <form action="create.php" method="post">
                Username
                <input type="text" name="username">

                Password
                <input type="password" name="password">

                Re-enter Password
                <input type="password" name="confirm_password">

                <button type="submit">Create Account</button>
            </form>

create.php

<!DOCTYPE html>

        <meta charset="UTF-8">
        <title>create.php</title>

        <?php

            $servername = "localhost";
            $username = "XXXXXXXXXX"; // Not shown
            $password = "XXXXXXXXXX"; // Not shown
            $dbname = "Database";

            //Create connection
            $mysqli = new mysqli($servername, $username, $password, $dbname);

            //Test connection
            if ($mysqli->connect_error) {
                die("Connection failed: " . $mysqli->connect_error);
            }

            $new_user_usr = filter_input(INPUT_POST, 'username');
            $new_user_pwd = filter_input(INPUT_POST, 'password');
            $new_user_pwd_conf = filter_input(INPUT_POST, 'confirm_password');

            $sql = "SELECT usr, pwd FROM Users";
            $result = $mysqli->query($sql);

            if($result->num_rows > 0) {

                /* If passwords don't match */
                if($new_user_pwd !== $new_user_pwd_conf) {
                    die("Passwords don't match");
                }

                /* If password isn't between bounds */
                if(strlen($new_user_pwd) <= 7 || strlen($new_user_pwd) >= 13) {
                    die("Password not long enough! Must be at least 8 characters long, but not greater than 12 characters");
                }

                /* If username is the same as password*/
                if($new_user_usr === $new_user_pwd) {
                    die("Username cannot equal password!");
                }

                while($row = $result->fetch_assoc()) {
                    if($row['usr'] === $new_user_usr) {
                        die("Username already taken");
                    }
                }

                $add = "INSERT INTO Users (usr, pwd) VALUES ('$new_user_usr', '$new_user_pwd')";

                echo $mysqli->query($add) ? "user created successfully" : "Error: " . $add . "
" . $mysqli->error;

            }

        ?>

I would like to kick old habits to the curb

That's a very good intention. Especially given that the present code, sadly, is rather a display of such old habits.

In order to improve, consider learning the following essentials:

PDO prepared statements (be advised that PDO is superior to mysqli for learners). Basically all your queries involving variables must be run not via query() but via prepare()/execute(), with all variables in the query substituted with placeholders.
Password hashing is essential. Just follow the simple example and store in the database not the original password but the result of password_hash() function's call.

there is no reason to limit the maximum password length. Let a user have the password as big as they wish.

the proper error reporting. first of all, always configure PHP to report errors by itself. And then never write a single line of code that intentionally outputs the error message. It's your server's configuration should be responsible for the errors' output.
Use a better code structure. Consider having both the HTML form and the handler sharing the same address, making the file with the form not called directly but included by the PHP file on demand. It will let you to utilize the POST/Redirect/Get pattern. Among other things it will let you to show errors nicely instead of bluntly aborting the script execution to show one. Collect all errors in the array and then run your query only if it is empty. Show the form back along the errors and the entered data otherwise.

the code used for the database connection is better to be stored in a separate file and then just included into every script that will need it.

avoid the code that does nothing useful. For example, running a select query to get all users from the database in order to create a user just makes no sense. You can ditch both the query and the condition that follows.

Taking all the above into consideration your code could be rewritten like this

db.php:

$host = '127.0.0.1';
$db   = 'Database';
$user = 'XXX';
$pass = 'XXX';
$charset = 'utf8mb4';

$options = [
    PDO::ATTR_ERRMODE            => PDO::ERRMODE_EXCEPTION,
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC,
    PDO::ATTR_EMULATE_PREPARES   => false,
];
$dsn = "mysql:host=$host;dbname=$db;charset=$charset";
try {
     $pdo = new PDO($dsn, $user, $pass, $options);
} catch (\PDOException $e) {
     throw new \PDOException($e->getMessage(), (int)$e->getCode());
}

create_user.php

$errors = [];
if ($_POST)
{
    require 'db.php';

    $username = filter_input(INPUT_POST, 'username');
    $password = filter_input(INPUT_POST, 'password');
    $pwd_confirm = filter_input(INPUT_POST, 'confirm_password');

    if ($password !== $pwd_confirm) {
        $errors[] = "Passwords don't match";
    }
    if (strlen($password) < 8) {
        $errors[] = "Password not long enough! Must be at least 8 characters long";
    }
    if ($username === $password) {
        $errors[] = "Username cannot equal password!";
    }

    $stmt = $pdo->query("SELECT 1 FROM Users WHERE usr = ?");
    $stmt->execute([$username]);
    $user_found = $stmt->fetchColumn();
    if ($user_found) {
        $errors[] = "Username already taken";
    }
    if (!$errors)
    {
        $hashed_password = password_hash($password, PASSWORD_DEFAULT);
        $stmt = $pdo->prepare("INSERT INTO Users (usr, pwd) VALUES (?, ?)");
        $stmt->execute([$username, $hashed_password]);
        header("Location: ."); // consider redirecting to the user profile
        exit;
    }
} else {
    $username = "";
}

include 'form.php';

form.php

<!DOCTYPE html>

    <meta charset="UTF-8">
    <title>create.html</title>

    <link rel="stylesheet" type="text/css" href="style.css">

    <?php foreach ($errors as $error): ?>

<?= $error ?>

    <?php endforeach ?>
    <form method="post">
        Username
        <input type="text" name="username" value="<?= htmlspecialchars($username) ?>">

        Password
        <input type="password" name="password">

        Re-enter Password
        <input type="password" name="confirm_password">

        <button type="submit">Create Account</button>
    </form>


Fixed-length Sequences in Swift 2.2 The following is an implementation of fixed-length sequences that makes very exotic (and hopefully fun) use of Swift 2.2 types. The question is what exactly is the cost of having the type checker guaranteeing sequence length in this way on safety? performance? readability?

protocol CountType {
    static var count: Int { get }
}

protocol DigitType : CountType {}

enum _0 : DigitType { static let count = 0 }
enum _1 : DigitType { static let count = 1 }
enum _2 : DigitType { static let count = 2 }
enum _3 : DigitType { static let count = 3 }
enum _4 : DigitType { static let count = 4 }
enum _5 : DigitType { static let count = 5 }
enum _6 : DigitType { static let count = 6 }
enum _7 : DigitType { static let count = 7 }
enum _8 : DigitType { static let count = 8 }
enum _9 : DigitType { static let count = 9 }

At this point we can do very little:

_4.count // 4
_2.count // 2

To express larger natural numbers, however, we must combine DigitTypes:

import CoreGraphics // TODO: wean off (implement `pow`)

extension DigitType {
    static func value(place place: Int) -> Int {
        return count * Int(pow(10, CGFloat(place)))
    }
}

enum __<T:DigitType, U:DigitType> : CountType { // these are two underscores `_`x2 (then, below, three, four...)
    static var count: Int {
        return T.value(place: 1) + U.value(place: 0)
    }
}
enum ___<H:DigitType, T:DigitType, U:DigitType> : CountType {
    static var count: Int {
        return H.value(place: 2) + T.value(place: 1) + U.value(place: 0)
    }
}
enum ____<Th:DigitType, H:DigitType, T:DigitType, U:DigitType> : CountType {
    static var count: Int {
        return Th.value(place: 3) + H.value(place: 2) + T.value(place: 1) + U.value(place: 0)
    }
}

... which allows us to count in types (try it in the playground):

____<_2,_0,_1,_6>.count // 2016

The point is that we can now make use of CountTypes to define sequence length:

final class Box<T> {
    var value: T
    init(_ value: T) { self.value = value }
}

struct Vector<Dimension: CountType, Element> : SequenceType {

    private let array: [Box<Element>]
    var elements: [Element] { return array.map{ $0.value } }

    subscript(index: Int) -> Box<Element> { return array[index] }

    init(_ repeatedValue: Element) {
        array = (0..<Dimension.count).map{ _ in Box(repeatedValue) }
    }

    func generate() -> Array<Element>.Generator {
        return elements.generate()
    }
}

// TODO: Matrix<Rows: CountType, Cols: CountType, Element>

We may now work with such Vectors and Matrixes with confidence that their dimension can never change:

let v = Vector<__<_4,_2>,Int>(0)
v.elements.count // 42

let w = Vector<_3,Int>(0)
w.elements // [0, 0, 0]
w[2].value = 5

for x in w {
    x // 0, 0, 5
}

(hm... the underscore syntax looks prettier in my playground...)

I was considering various alternative naming schemes for the combining enums. So, instead of underscores, we could perhaps emulate the exponent syntax for number literals (like 0.42e2 meaning 0.42 * pow(10, 2)). This would work out as following:

e4<_2,_0,_1,_6>.count // 2016

// and

let v = Vector<e2<_4,_2>,Int>(0)

The reason I decided against this and similar approaches is that there is now a rather irrelevant number 2 appearing alongside _4,_2...

The Vector type is not related to graphics, so importing
CoreGraphics seems "unnatural" here. If you replace CGFloat
by Float or Double

extension DigitType {
    static func value(place place: Int) -> Int {
        return count * Int(pow(10, Double(place)))
    }
}

then it suffices to import Foundation (or Swift + Darwin).

But I would get rid of the floating point conversion and implement it as (taking the hundreds as an example):

enum ___<H: DigitType, T: DigitType, U: DigitType> : CountType {
    static var count: Int {
        return H.count * 100 + T.count * 10  + U.count
    }
}

This is simple and straightforward, and easy to read. This makes the
value(place place:) with its floating point conversion, and the
the math library call to the pow() function obsolete.

As a consequence, the entire code compiles with a pure

import Swift

without additional imports.

(This is my major point.) The boxing of the array elements into
a class seems problematic to me.

First, struct Vector is a value type, but its elements are
mutable even for a constant:

let vecA = Vector<_3, Int>(0)
vecA[0].value = 111 // This should not compile!

Second, copying a vector copies references to the elements,
which is not to be expected for a value type:

let vecA = Vector<_3, Int>(0)
vecA[0].value = 111

let vecB = vecA
vecB[0].value = 222

print(vecA[0].value) // Output: 222. Expected output: 111.

Here, modifying the elements of vecB modifies the elements of
vecA as well.

I would simply omit the boxing and store the elements just as an
array. Modifying the elements is then done via a subscript setter:

struct Vector<Dimension: CountType, Element> {

    private var array: [Element]

    init(_ repeatedValue: Element) {
        array = [Element](count: Dimension.count, repeatedValue: repeatedValue)
    }

    var elements: [Element] {
        return array
    }

    subscript(index: Int) -> Element {
        get {
            return array[index]
        }
        set(newValue) {
            array[index] = newValue
        }
    }

    func generate() -> Array<Element>.Generator {
        return array.generate()
    }
}

Using a private var array for the element storage and a
computed elements property still makes sense for encapsulation.

Now Vector behaves like a proper value type:

var vecA = Vector<_3, Int>(0)
vecA[0] = 111

var vecB = vecA
vecB[0] = 222

print(vecA[0]) // Output: 111

By adopting the CustomStringConvertible protocol,

extension Vector : CustomStringConvertible {
    var description: String {
        return "<" + (array.map { String($0) }).joinWithSeparator(", ") + ">"
    }
}

printing a vector produces nicer output (the angle brackets are just
an example to disambiguate the output from an array):

print(vecA)
// Output: <111, 0, 0>
// instead of Vector<_3, Int>(array: [111, 0, 0])

To create (constant) vectors, an init method taking a "index to
element mapping" might be useful:

extension Vector {
    init(elementForIndex: Int -> Element) {
        array = (0 ..< Dimension.count).map(elementForIndex)
    }
}

Example:

let vecC = Vector<_5, Int>(elementForIndex: { 2 * $0 })
print(vecC) // <0, 2, 4, 6, 8>

This would also come in handy when you start to define operations
on the vector type, e.g. an addition of integer vectors:

func + <Dimension, Element: IntegerArithmeticType>
    (lhs: Vector<Dimension, Element>, rhs: Vector<Dimension, Element>)
    -> Vector<Dimension, Element> {
        return Vector(elementForIndex: { lhs[$0] + rhs[$0]})
}

Initialization from an array or array literal might also be
convenient:

extension Vector {
    init(elements: [Element]) {
        precondition(elements.count == Dimension.count)
        array = elements
    }
}

extension Vector: ArrayLiteralConvertible {
    init(arrayLiteral elements: Element...) {
        self.init(elements: elements)
    }
}

Example:

let vecD: Vector<_5, Double> = [ 1.0, 2.0, 3.0, 4.0, 5.0 ]
print(vecD) // <1.0, 2.0, 3.0, 4.0, 5.0>

The disadvantage here is that the compiler cannot check the
dimension.


Deduplicating value pairs between 2 columns This was asked in the SQL Helpline room:

I have a sql query concern. Could you help me?
It goes like this. I have a table with the Records

A B
A C
B A
B C
C A
C B

How do I get only the records that there aren't duplicated in both columns

A B
A C
B C

While it seemed simple at first, it was surprisingly difficult to get it to work. In the end, I had to resort to a loop. Please mind, this looks like example code, but it is more like practice code, so that the logic can be applied to real situations.

I saved a demo here (which uses temp tables) and the query goes like this:

/* Assuming that @Origin is the source table your data currently is: */
declare @Origin table (val1 char(1), val2 char(1));
insert into @Origin (val1, val2)
values
  ('A', 'B'),
  ('A', 'C'),
  ('B', 'A'),
  ('B', 'C'),
  ('C', 'A'),
  ('C', 'B');

/* Create a temp table in memory:
    Note: We could also use a physicial #holdingTable
    if the result set is too large for memory */
declare @holdingTable table (
  rownum int primary key,
  val1 char(1),
  val2 char(1)
);
insert into @holdingTable (
    rownum,
    val1,
    val2
)
select
  /* generate a rownum to use for looping: */
  row_number() over (order by val1, val2),
  /* add values from origin table: */
  val1,
  val2
from @Origin;

/* loop variables: */
declare @i int = (select min(rownum) from @holdingTable);
declare @max int = (select max(rownum) from @holdingTable);

/* deduplication variables: */
declare @val1Deduplicator char(1);
declare @val2Deduplicator char(1);

/* table to hold result set until end of query: */
declare @resultSet table (val1 char(1), val2 char(1));

while @i <= @max
begin
    /* assign values to deduplicate across columns: */
    select
        /* inversion between val1 and val2 happens here: */
        @val1Deduplicator = val2,
        @val2Deduplicator = val1
    from @holdingTable
    where rownum = @i;
    /* verify we are not inserting a duplicate: */
    if not exists (
        select 1
        from @resultSet
        where val1 = @val1Deduplicator
          and val2 = @val2Deduplicator
    )
    /* if confirmed not a duplicate, insert the values from the current row: */
    begin
        insert into @resultSet (val1, val2)
        select val1, val2
        from @holdingTable
        where rownum = @i;
    end;
    /* increment loop: */
    set @i = @i + 1;
end;
/* obtain the final results: */
select * from @resultSet;

The performance seems OK for this few records, but it's not set-based and might get really slow for a large number of records. Is there a smarter way to do this? Is there anything else I can improve?

Output is correct:

val1 val2
---- ----
A    B
A    C
B    C

(3 row(s) returned)

If val1 and val2 are comparable it is easy to find the distinct values, using set-based operations, by normalizing first and then taking distinct rows.

Given:

CREATE TABLE [TestTable]
(
    [TestID] INT NOT NULL IDENTITY(1,1),
    [Value1] NCHAR(1) NOT NULL,
    [Value2] NCHAR(1) NOT NULL,
)

INSERT INTO [TestTable]
    ([Value1], [Value2])
VALUES
    (N'A', N'B'),
    (N'A', N'C'),
    (N'B', N'A'),
    (N'B', N'C'),
    (N'C', N'A'),
    (N'C', N'B'),
    (N'D', N'C');

If you just need the values, and not the actual rows, you can use SELECT DISTINCT with a normalized CTE:

WITH [Normalized] AS
(
    SELECT
        CASE WHEN [tt].[Value1] > [tt].[Value2]
            THEN [tt].[Value2]
            ELSE [tt].[Value1] END [N1],
        CASE WHEN [tt].[Value1] > [tt].[Value2]
            THEN [tt].[Value1]
            ELSE [tt].[Value2] END [N2]
    FROM
        [TestTable] [tt]
)
SELECT DISTINCT
    [n].[N1] [Value1],
    [n].[N2] [Value2]
FROM
    [Normalized] [n];

Result:

Value1 Value2
------ ------
A      B
A      C
B      C
C      D

If you need the original rows instead you have to eliminate the duplicates yourself. This can be done using the normalized CTE and partitioning by the normalized values:

WITH [Normalized] AS
(
    SELECT
        CASE WHEN [tt].[Value1] > [tt].[Value2]
            THEN [tt].[Value2]
            ELSE [tt].[Value1] END [N1],
        CASE WHEN [tt].[Value1] > [tt].[Value2]
            THEN [tt].[Value1]
            ELSE [tt].[Value2] END [N2],
        [tt].[TestID],
        [tt].[Value1],
        [tt].[Value2]
    FROM
        [TestTable] [tt]
),
[Ranked] AS
(
    SELECT
        [n].[TestID],
        [n].[Value1],
        [n].[Value2],
        ROW_NUMBER() OVER (PARTITION BY [n].[N1], [n].[N2]
                           ORDER BY [n].[TestID] ASC) AS [Rank]
    FROM
        [Normalized] [n]
),
[Distinct] AS
(
    SELECT
        [r].[TestID],
        [r].[Value1],
        [r].[Value2]
    FROM
        [Ranked] [r]
    WHERE
        [r].[Rank] = 1
)
SELECT [d].[TestID], [d].[Value1], [d].[Value2]
FROM
    [Distinct] [d];

Result:

TestID      Value1 Value2
----------- ------ ------
1           A      B
2           A      C
4           B      C
7           D      C

As for performance, the iterative approach is, according to the execution plan in SSMS14, about 10 times slower. The set based solution is a table scan + a sort where as the execution plan for the iterative solution is a mess to look at.


Get all combination of a nested object I have JSON data which can be an object/array of recursively nested object or array. The value of an array can not be null, but a value in an object can be null. And I would like to return all combinations of these keys and values through depth-first search.

For example:

var data = {
  "title": {
     "original": "Hitchhiker",
     "more": ["HGTTG"],
     "link": null
  },
  "date": ["20150101", "20160101"]
}

The data's length and depth is arbitrary, and the combination result I want is something like this:

["title-original-Hitchhiker", "title-more-HTTG", "title-link", "date-20150101", "date-20160101"]

What I came up with is recursive:

function nestedObjectToArray(obj) {
  if (typeof(obj) != "object"){
    return [obj];
  }
  var result = [];
  if (obj.constructor == Array){
    for (var i = 0; i <obj.length; i++){
      if (obj[i]){
        var temp = nestedObjectToArray(obj[i]);
        for (var j = 0; j < temp.length; j++){
          result.push(temp[j]);
        }
      }
    }
  } else {
    for (var i in obj){
      if (obj.hasOwnProperty(i)) {
        if (obj[i] == null){
          result.push(i);
        } else {
          var temp = nestedObjectToArray(obj[i]);
          for (var j = 0; j < temp.length; j++){
            result.push(i+"-"+temp[j]);
          }
        }
      }
    }
  }
  return result;
}

Would you have a better/more elegant solution than this chunk of conditions, loops and recursion? I can use any library if it's more convenient.

A little refactor solution can be

function nestedObjectToArray(obj) {
    if (typeof(obj) !== "object"){
        return [obj];
    }
    var result = [];
    if (obj.constructor === Array){
        obj.map(function(item) {
            result = result.concat(nestedObjectToArray(item));
        });
    } else {
        Object.keys(obj).map(function(key) {
            if(obj[key]) {
                var chunk = nestedObjectToArray(obj[key]);
                chunk.map(function(item) {
                    result.push(key+"-"+item);
                });
            } else {
                result.push(key);
            }
        });
    }
    return result;
}