Zoom in/zoom out jQuery plugin I created my first plugin in jQuery and just want some inputs on how I can improve the code and features.

Here is the fiddle

    $.fn.fruitZoom = function (options) {

var defaults = {
    ele: "p,span",
};

var settings = $.extend(defaults, options);
$("#fruitZoom :first-child").click(function () {

    fontSize = $(defaults.ele).css("font-size")
    $(defaults.ele).css("font-size", parseInt(fontSize) + 2);
    console.log(fontSize)
});
$("#fruitZoom :last-child").click(function () {
    fontSize = $(defaults.ele).css("font-size")
    $(defaults.ele).css("font-size", parseInt(fontSize) - 2);
    console.log(fontSize)
});
};
})(jQuery);
options = {
    ele: "p,h1",
}
$("#fruitZoom").fruitZoom(options);

Flexibility

Your plugin relies on very specific markup; an element with an ID of fruitZoom having a first and last child element meant to be used as zoom in and zoom out buttons, respectively.

This is not flexible enough to be generally useful. What if you want multiple zoom controls on the page at once? What if you want to use a slider instead of buttons, or keyboard controls?

Naming and style

The identifier ele is not appropriate for a user-facing option name. The name elements would be an improvement. Better yet, use something more descriptive of what the value should to be, such as selector. After all, the user does not assign elements to this property, they assign a selector to it.

Your use of semicolons is inconsistent. Don't rely on automatic semicolon insertion in some places and not in others. Either terminate every statement with a semicolon, or only use them as separators in places where they are necessary. Pick a style and stick to it.

Behavior

This line here...

fontSize = $(defaults.ele).css("font-size")

...only gets the size of the first element matched by the ele selector. That means as soon as you zoom in or out, all elements matching your selector are resized to the size of the first element. You can see this in your own example code on JSFiddle; as soon as you zoom, the heading changes its size to match the first paragraph. This is probably not desirable behavior.

Redundancy

This block of code...

$("#fruitZoom :first-child").click(function () {
    fontSize = $(defaults.ele).css("font-size")
    $(defaults.ele).css("font-size", parseInt(fontSize) + 2);
    console.log(fontSize)
});

...is nearly identical to the next:

$("#fruitZoom :last-child").click(function () {
    fontSize = $(defaults.ele).css("font-size")
    $(defaults.ele).css("font-size", parseInt(fontSize) - 2);
    console.log(fontSize)
});

This is a sure sign that you should refactor your code to eliminate redundancy. Consider writing a named function, replacing the + 2 / - 2 part with + amount, where amount is a variable you pass in, having a value of 2 or negative 2.


Understanding htaccess redirect to HTTPS and WWW I have some questions about a code I found on stackoverflow. The code does what I want to do. Redirects to HTTPS and WWW when doesn't exist in URL. But, I want now to understand what I'm doing.
So, here is the code I use:
# Redirect non HTTPS to HTTPS #
RewriteCond %{HTTPS} !=on
RewriteRule ^(.*)$ https://%{HTTP_HOST}/$1 [R=301,L]

# Force www #
RewriteCond %{HTTP_HOST} !^www\.
RewriteRule ^(.*)$ https://www.%{HTTP_HOST}/$1 [R=301,L,E=HTTPS:1]

The code above works as expected. Why isn't the code in the next format, as it does the same thing ? Is any difference ?

RewriteCond %{HTTPS} !=on [OR]
RewriteCond %{HTTP_HOST} !^www\.
RewriteRule ^(.*)$ https://www.%{HTTP_HOST}/$1 [R=301,L]

The first line of code looks a little bit ambiguous. So I try to modify with:

RewriteCond %{HTTPS} != on # Put a space before on

Or:
RewriteCond %{HTTPS} != 'on' # Put on in single quotes

Or:
RewriteCond %{HTTPS} != "on" # Put on in double quotes

And it doesn't work. Can you please explain why ?

I found some users which are using:

RewriteCond %{HTTPS} off

and not:
RewriteCond %{HTTPS} !=on

I understand that one line checks if the https is off and another one if https is not on. We can say they do the same thing. There are other differences here ? Can HTTPS not be neither on or off ? If the answer is yes, that means the line RewriteCond %{HTTPS} !=on is better than RewriteCond %{HTTPS} off. Isn't it ?
4 When forcing www I have E=HTTPS:1 flag, which means I set the variable HTTPS with value 1. I don't understand the reason... When and who checks for this variable ?
Here, on question 4 I try to rename HTTPS to HTTPSaaa and then, with php, to get all server variable (from $_SERVER). I don't find HTTPSaaa. So this means that the flag is used only by htaccess ? So, I ask again. What's the reason for this flag/variable to exist ?
PS: If someone want's to answer, please specify the question number. I am already a little bit confused...

The code above works as expected. Why isn't the code in the next format, as it does the same thing? Is any difference ?
RewriteCond %{HTTPS} !=on [OR]
RewriteCond %{HTTP_HOST} !^www\.
RewriteRule ^(.*)$ https://www.%{HTTP_HOST}/$1 [R=301,L]

This code is different to your "working" code (at the top of your question). In this code, the www subdomain is always prefixed to the requested hostname. So, if you were to request http://www.example.com/foo (HTTP + www) then it will redirect to https://www.www.example.com/foo - which is presumably not the intention.
This can be "fixed" with an additional condition in order to capture the hostname less the "optional" www subdomain, however, this is still not "the same" as the "working" directives you posted. For example:
RewriteCond %{HTTPS} !=on [OR]
RewriteCond %{HTTP_HOST} !^www\.
RewriteCond %{HTTP_HOST} ^(?:www\.)?(.+?)\.?$ [NC]
RewriteRule ^(.*)$ https://www.%1/$1 [R=301,L]

The only purpose of the 3rd condition is to capture the hostname, less the www. prefix (if any). This condition is always successful. The hostname (less the www. prefix) is stored in the %1 backreference.
Just to emphasise... this is not exactly the same as the "working" redirects you posted initially. This performs the HTTPS + www redirect in 1 go, instead of two (potentially) in your "working" directives. eg. When you request HTTP and non-www then your "working" redirects will trigger 2 redirects, first to HTTPS on the same host*1 and then to HTTPS + www.
*1 This is a necessary requirement if you have implemented HTTP Strict Transport Security (HSTS) - where you must redirect to HTTPS on the same host before canonicalising the hostname. So, one redirect is not always "better" than two.

The first line of code looks a little bit ambiguous. So I try to modify with:
RewriteCond %{HTTPS} != on # Put a space before on

It may look ambiguous to you, but I assure you it's not.
You cannot simply inject spaces here, since spaces are argument delimiters in Apache config files. So, by putting a space after the != you are now passing 3 arguments to the RewriteCond directive: %{HTTPS}, != and on. on is likely to trigger an "invalid flags" error, since this will be interpreted as the 3rd "flags" parameter (eg. normally something like [NC]).
The = and ! are prefix-operators and are in fact part of the argument (CondPattern). They change the way the CondPattern (2nd argument) is interpreted. != is not a single operator, it does not strictly mean not-equals, as you are perhaps expecting, although the net result is the same. (More on this below).
The only time you need to use double-quotes (not single-quotes) around an argument in Apache config files is if the argument contains spaces (since, as mentioned above, spaces are argument delimiters). Although you can always surround the argument in double-quotes if you wish. And you can sometimes backslash escape the space instead (if the argument is a regex).
But note, the double-quotes surround the entire argument (which includes the prefix operators). For example:
RewriteCond %{HTTPS} "!=on"

Aside: Apache does not support line-end comments, so the # Put a space before on at the end of the line is not valid. Due to the way Apache directives are processed, this may appear to be OK at times (ie. no error), but it is strictly invalid. (You may have only used this in the code sample in your question, however, it is a common source of error, so worth mentioning.)

I found some users which are using:

RewriteCond %{HTTPS} off

and not:
RewriteCond %{HTTPS} !=on

I understand that one line checks if the https is off and another one if https is not on. We can say they do the same thing. There are other differences here ?

I've actually answered the same question on the Webmasters stack previously, so I'll just link to that for more detail:

https://webmasters.stackexchange.com/questions/118391/in-htaccess-files-is-there-any-difference-at-all-between-on-and-off

But yes, the net result is indeed the same and it's primarily personal preference which method you choose. Although the two methods are technically different.

off is a regex comparison: Does off occur anywhere in the value of the HTTPS server variable?
!=on is not a regex comparison. The = prefix changes it to a lexicographical string comparison. ie. Is the value of the HTTPS server var exactly equal to "on"? The ! prefix then negates the expression. So the condition is successful when it does not match.

Can HTTPS not be neither on or off ?

The HTTPS server variable can only be "on" or "off" if your server is correctly configured.

If the answer is yes, that means the line RewriteCond %{HTTPS} !=on is better than RewriteCond %{HTTPS} off. Isn't it ?

Not necessarily. If HTTPS is not "on" or "off" then it's more likely to not be set at all (on all requests), so !=on would always be successful and result in a redirect loop. Whereas off will simply fail. Although this is hypothetical, your server would have to be "broken" for the HTTPS server variable not to be set at all and to not be "on" or "off".

When forcing www I have E=HTTPS:1 flag, which means I set the variable HTTPS with value 1.

First, a bit of clarification... there are 2 variables here:

The HTTPS server variable as set by Apache and referenced in %{HTTPS}. This is effectively readonly.
The HTTPS environment variable that you are setting here. This would be readable using the syntax %{ENV:HTTPS}.

Whilst it is relatively common to see this, it is arguably confusing to have two different variables of the same name. It would be better to change your environment variable to something like HSTS instead (see below), to avoid any confusion.

I don't understand the reason... When and who checks for this variable ?

Well, in the directives you posted it's not required. So, maybe this is not required here? But...
The reason for setting an env var on the HTTPS redirect like this is a common pattern when implementing HSTS*2. When implementing HSTS you need to set the Strict-Transport-Security HTTP response header on the HTTPS to HTTPS redirect itself. This is why I suggested using a variable called HSTS instead. If you are not setting this header with a conditional Header directive elsewhere in your config file (based on this env var) then you are not implementing HSTS (properly) and this env var is most probably superfluous.
*2 Although I wouldn't necessarily do it this way myself. I would set the env var in a separate rule, since you still need to set the STS header on other HTTPS responses, not just the redirect. See my answer to the following question on the Webmasters stack for more information on this, with example code:

https://webmasters.stackexchange.com/questions/115125/hsts-implementation-when-using-www-tld

Here, on question 4 I try to rename HTTPS to HTTPSaaa and then, with php, to get all server variable (from $_SERVER). I don't find HTTPSaaa. So this means that the flag is used only by htaccess ? So, I ask again. What's the reason for this flag/variable to exist ?

Yes, it's only used by Apache/.htaccess. It is set only on the redirect response back to the client. Your PHP script is not processed in this time, so cannot read it.
The reason would be to help with implementing HSTS, as described above.

If you are interested in HSTS then see this related CodeReview question (and my answer) regarding the implementation of HSTS in .htaccess:

HSTS Recommendations in .htaccess


Does checking for the Core.Instance.isLocalPlayer twice in my method constitute a bad design? I've been writing some code for my unity project, where I am connecting players to a game and adding them to a dictionary, which is in a Database class.

I have a simple method which is adding them to the database and if successful, it then instantiates the relevant game objects into the game.

The thing that made me wonder if it was a bad design was the way I was checking if the player is the local client or not.

This is my code:

public bool SetConnection(byte connectionID)
{
    Agent agent;
    if (Core.Instance.isLocalPlayer)
        agent = new Agent_Local();
    else
        agent = new Agent_Enemy();

    if (Core.Instance.Database.Add(connectionID, agent)) // fails if room is full
    {
        if (Core.Instance.isLocalPlayer) //Checking this again : maybe bad design?
            Core.Instance.LocalHandler.SetPlayer(agent);
        else
            Core.Instance.SetEnemy(agent);

        return true;
    }
    return false;
}

So you will notice I am checking if the player is local twice, and I feel that I shouldn't need to check if I was smarter about the way I went about setting this up. Is this considered as bad or redundant coding or perhaps someone could suggest a better approach?

If you find yourself using a lot of if (type == x) do x else do y statements you can use inheritance. The problem with using if (type == x) throughout your code is that if tomorrow you add a new Agent, you have to add an extra else case in each place you do this.

Here's an example of how to improve that:

public bool SetConnection(byte connectionID)
{
    Agent agent = agentFactory.CreateAgent(Core.Instance.IsLocalPlayer);
    return agent.TryToEnterRoom(connectionId);
}


Django template block name changing function I created a reusable function for changing block name in Djago templates. I want to know if the code is good or is there any probable issues in it. I developed the function as a hack for the following issue.

Template Code

{% extends base_name %}

{% block main-contents %}

{{ message_heading }}

        {{ message }}

        {% if btn_1_text and btn_1_url %}
            {{ btn_1_text }}
        {% endif %}

        {% if btn_2_text and btn_2_url %}
            {{ btn_2_text }}
        {% endif %}

{% endblock %}

Function to change block name

def change_block_names(template, change_dict):
    """
    This function will rename the blocks in the template from the
    dictionary. The keys in th change dict will be replaced with
    the corresponding values. This will rename the blocks in the
    extended templates only.
    """

    extend_nodes = template.nodelist.get_nodes_by_type(ExtendsNode)
    if len(extend_nodes) == 0:
        return

    extend_node = extend_nodes[0]
    blocks = extend_node.blocks
    for name, new_name in change_dict.items():
        if blocks.has_key(name):
            block_node = blocks[name]
            block_node.name = new_name
            blocks[new_name] = block_node
            del blocks[name]

tmpl_name = 'django-helpers/twitter-bootstrap/message.html'
tmpl1 = loader.get_template(tmpl_name)
change_block_names(tmpl1, {'main-contents': 'new-main-contents'})
return HttpResponse(tmpl.render(Context()))

This function seems to work for my issue. But I don't know if this breaks any of the Django's functionality. Can anyone verify this and give me some advice?

This seems valid, and I don't think it should mess up Django's functionality.
Some improvements are possible.

This is not Pythonic:

if len(extend_nodes) == 0:
    return

Write like this:

if not extend_nodes:
    return

This is not Pythonic:

    if blocks.has_key(name):
        # ...

Write like this:

    if name in blocks:
        # ...

But...

You're not supposed to do this.
You haven't explained why you want to change template block names on the fly.
And I cannot think of a legitimate use case.
It's inefficient to do this operation on the fly,
for every page load.
What you should really do instead is update the template files in your project,
do the necessary renames once and save it,
so that you don't need this unnecessary extra operation.
The end result will be more efficient, and more robust.
Updating all files in your project is possible with a little scripting.


Async SerialPort Wrapper I've been working with the SerialPort class for a while, trying to figure out the best way to work with it, and especially adding support for Async-Await in C#. I also started before 4.5, so I didn't have access to the BaseStream member (so no BaseStream.ReadAsync()). I wanted to get some opinions on it before I started relying on it too heavily. Performance considerations and stability are high priority, but also figuring out if I'm reinventing the wheel.

The code is designed to wrap around a SerialPort to provide Async-Await operations. It does this by intercepting calls to Write and the SerialPort.DataRecieved events and relaying the data to a pair of Reactive Subjects that client code can subscribe to. Note this code requires the nuget package Rx-Main.

using System.IO.Ports;
using System.Reactive;
using System.Reactive.Linq;
using System.Reactive.Subjects;
namespace Core.IO {
    public sealed class ComPort : IDisposable, INotifyPropertyChanged{
        private Subject<byte[]> _txDataSubject;
        private Subject<byte[]> _rxDataSubject;
        private SerialPort _port;

        public String PortName {
            get {
                return _port.PortName;
            }
            set {
                _port.PortName = value;
                RaisePropertyChanged("PortName");
                RaisePropertyChanged("IsOpen");
            }
        }
        public Boolean IsOpen {
            get {
                return _port.IsOpen;
            }
        }
        public IObservable<byte[]> DataTx {
            get {
                return _txDataSubject.AsObservable();
            }
        }
        public IObservable<byte[]> DataRx {
            get {
                return _rxDataSubject.AsObservable();
            }
        }
        public ComPort() {
            _port = new SerialPort();
            _txDataSubject = new Subject<byte[]>();
            _rxDataSubject = new Subject<byte[]>();
            _port.DataReceived+=(sender,args)=>{
#if DEBUG
                Console.WriteLine("({0}) RX on thread {1}",PortName, System.Threading.Thread.CurrentThread.ManagedThreadId);
#endif
                _rxDataSubject.OnNext(_port.Encoding.GetBytes(_port.ReadExisting()));
            };
        }
        public void Open() {
            _port.Open();
            RaisePropertyChanged("IsOpen");
        }
        public void Close() {
            _port.Close();
            RaisePropertyChanged("IsOpen");
        }
        private void RaisePropertyChanged(string propertyName) {
            if (PropertyChanged != null) {
                PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
            }
        }
        public event PropertyChangedEventHandler PropertyChanged;
        public Boolean IsDisposed {
            get;
            private set;
        }
        public void Write(string data) {
            _txDataSubject.OnNext(_port.Encoding.GetBytes(data));
            _port.Write(data);
        }
        private void Dispose(bool isDisposing) {
            if (IsDisposed) return;
            if (isDisposing) {
                _port.Dispose();
                _txDataSubject.Dispose();
            }
            IsDisposed = true;
        }
        public void Dispose() {
            Dispose(true);
            GC.SuppressFinalize(this);
        }
        ~ComPort() {
            Dispose(false);
        }
    }
}

_port.Encoding.GetBytes(_port.ReadExisting())

If you want to work with byte[], then your data should never appear as string, even temporarily, because some byte sequences are likely going to be invalid in your encoding.

I believe you should be able to use the byte[] overload of Read() for this. (Though switching to byte[] and async methods of the BaseStream is probably a better choice in the long run.)

Your class is schizophrenic about whether it wants to use string or byte[] (the observables use byte[], Write() uses string). You should figure out which choice makes more sense and stick to it.

What is the purpose of DataTx? I think that being able to read what you have written is not commonly needed.

I realize that Tx and Rx are abbreviations used in networking, but I find them confusing. Are you sure users of this library will immediately understand them? If not, you should rename the properties.

Boolean
String

It's common to write those types in their keyword form: bool, string.

~ComPort() {
    Dispose(false);
}

You don't have any unmanaged resources, so Dispose(false) basically doesn't do anything. But this means there is no reason for this class to have a finalizer.

The Dispose Pattern makes sense when you're writing a class that is going to be inherited. But since your class is sealed (nice, most people don't bother with that), I think there is also no reason to have the Dispose(bool) overload, just a single Dispose() method will be enough.


Bot clean written on C++ First string includes 4 integers: N, M, A, B (1 ≤ N, M ≤ 300, A, B ≤ 1000).
Each next line include M-symbols. Symbol . is a clean cell, * and ** are dirty.

I need to find the sum for cleaning if A its sum of ** cell, B is sum of *.

What I can do better?

#include <iostream>
#include <string>
#include <vector>

using std::cin;
using std::cout;
using std::string;
using std::vector;

int main() {
  int n, m, a, b;
  cin >> n >> m >> a >> b;

  vector<string> parquet;

  int itr = 0;

  while (itr < n) {
    string cell;
    cin >> cell;
    parquet.push_back(cell);
    itr++;
  }

  int sum = 0;

  itr = 0;

  while (itr < parquet.size()) {
    for (int i = 0; i < parquet[itr].size(); i++) {
      if (parquet[itr][i] == '*') {
        if (parquet[itr][i + 1] == '*') {
          i++;
          sum += a;
        } else {
          sum += b;
        }
      }
    }

    itr++;
  }

  cout << sum;
}

You can refine your algorithm by getting rid of all these counters and while loops and replace them with a for loop such as this:

for (int i = 0; i < parquet.size(); i++)
    parquet[i] = ... // do whatever

Similarly, you can re-write the summing algorithm as:

for (int i = 0; i < parquet.size(); i++){
    for (int j = 0; j < parquet[i].size(); j++) {
      if (parquet[i][j] == '*') && (parquet[i][j + 1] == '*') {
          j++;
          sum += a;
      else
          sum += b;
      }
    }
}

That said, I notice you're naming your counters as "iter" (which I assume is a short for iterator). Are you trying to access your vector in a typical STL manner (i.e.: using iterators?). If that was your intent, you can try something like:

vector<string>::iterator myIterator;
for (myIterator = parquet.begin(); myIterator != parquet.end(); myIterator++)
// Do whatever

That is, of course, assuming your procedure works as intended. If not, please state otherwise.


Duplicate counter in C My goal was to create a function that could check for duplicates in an array no matter what the data type was, given that the size of the data type was provided. This was done by using the \$\mathcal{O}(n^2)\$ algorithm and casting to char because of its 1 byte property.

/**
 * Returns count of duplicates in an array
 * @param  a     array to check in
 * @param  items how many entries in array
 * @param  size  size of each entry
 * @return       returns amount of duplicates/conflicts
 */
int dubCount(void *a, size_t items, size_t size){
    int i,j;
    int conflicts = 0;
    char *x = (char *)a;

    for (i = 0; i < items - 1; i++){
        for (j = i + 1; j < items; j++){
            /* Check if chunks are equal, if so count */
            if(memcmp(&(x[i*size]), &(x[j*size]), size) == 0){
                conflicts++;
            }
        }
    }

    return conflicts;
}

The code has been confirmed to work with many data types.
As I'm quite new to the C world, I'd like for someone to point out anything wrong or dangerous in this function.

This is a very straightforward implementation which is great for maintainability and readability. Here are some things you could improve:

Not All Comparisons Work In Obvious Ways

As mentioned in the comments by Jerry Coffin and Loki Astari, you may run into issues with floating point values or values which may not be padded how you think, or for which the padding contains random values. You could solve this by also passing in a pointer to a comparison function and calling that function to do the comparison.

Naming

Your naming could be improved a little. The function is named dubCount(), but from the comments and the name of the return value it should probably be conflictCount() or at least dupeCount(). It's not clear what dub refers to in dubCount() (at least to me). Also the parameter name a tells me nothing, and since it's a pointer to void, I have no contextual clue, either. I'd rename it to something like items and rename the items parameter to numberOfItems or numItems, and rename size to itemSize or sizePerItem or bytesPerItem so that it's more clear just from the function prototype what the parameters mean.

Use const where appropriate

In this function, none of the parameters to the function are modified. When your function doesn't modify a parameter, it should be marked as const. This lets both the compiler and a reader know the function won't change those values. The compiler may be able to make some optimizations with that info, and a reader can do more reasoning about the function without reading its actual code, which is helpful.

Performance

You noted that the algorithm you're using is \$\mathcal{O}(n^2)\$. If you sorted the items in the array first, you could use an O(n) search on the result as you'd be able to simply run through the array until you hit a different value. (Note that this may depend on the statistical nature of your input data and the sort algorithm you use. But in general, I'd expect to see a speedup in such a case.) Most fast sort algorithms are \$\mathcal{O}(n * log(n))\$ so the result would be \$\mathcal{O}(n + n * log(n))\$ (I think? Check my math). Note that you would use the same comparison function for the sort as you would for checking for duplicates.


Counting senders in a mailbox I'm learning Python from Coursera and am doing a class. One of the assignments is as follows:

  Write a program to read through the mbox-short.txt and figure out who has the sent the greatest number of mail messages. The program looks for 'From ' lines and takes the second word of those lines as the person who sent the mail. The program creates a Python dictionary that maps the sender's mail address to a count of the number of times they appear in the file. After the dictionary is produced, the program reads through the dictionary using a maximum loop to find the most prolific committer.

I have completed this assignment and I think I did pretty good.  How can I make it better?

def get_email(filename, name):
    count = 0
    user = {}
    handle = open(filename)

    for line in handle:
        if name in line and 'From' in line and 'From:' not in line:
            arr = line.split(' ')
            count += 1
            user['email'] = count

    email = ''.join(arr[1])
    print "%s %d" % (email, count)

get_email('mbox-short.txt', 'cwen')
#<= cwen@iupui.edu 5
get_email('mbox-short.txt', 'david')
#<= david.horwitz@uct.ac.za 4

Full text file to read from here

In some sense, you haven't really completed the task, which is to find the most prolific commenter.  Your get_email() function requires you to know the names before you call the function, and to know what those names are, you would have to read the mailbox!

Furthermore, your user dictionary is kind of pointless.  It's only ever used as a write-only variable user['email'].  And since only one key is used, there is no advantage to using a dictionary rather than just a regular variable.

To do the job properly, you should make one pass through the file, keeping track of the counts of all of the users as you go.  Using a dictionary to accomplish that is the point of the exercise.


A binary tree , its copy constructor and assignment operator I implemented a binary tree in the following code. Its node's copy constructor and assignment operator should copy itself and all its descendents. Similarity a node's destructor should delete itself and all nodes descended from it.The print function just prints each node in a new line. How can I pretty-print it? Please point to errors(if any) and suggestions.

#include <iostream>

struct Node
{
    Node(int);
    Node(const Node&);
    Node& operator=(const Node&);
    ~Node();

    Node* left;
    Node* right;
    int value;
};

Node::Node(int v)
:left(nullptr),
right(nullptr),
value(v)
{}

Node::Node(const Node& other)
:left(nullptr),
 right(nullptr),
 value(other.value)
{
    if (other.left != nullptr )
    {
        left = new Node(*other.left);
    }
    if (other.right != nullptr)
    {
        right = new Node(*other.right);
    }
}

Node& Node::operator=(const Node& other)
{
    value = other.value;

    Node * left_orig = left;
    left = new Node(*other.left);
    delete left_orig;

    Node * right_orig = right;
    right = new Node(*other.right);
    delete right_orig;

    return *this;
}

Node::~Node()
{
    if (left != nullptr )
    {
        delete left;
    }
    if (right != nullptr)
    {
        delete right;
    }
}

Node* make_copy(Node* other)
{
    Node * new_node = new Node(*other); // copy constructor invoked
    return new_node;
}

void print(Node* n)
{
    if (n == nullptr)
    {
        return;
    }
    std::cout << n << std::endl;
    print(n->left);
    print(n->right);

}

int main()
{
    Node* n = new Node(1);

    n->left = new Node(2);
    n->right = new Node(3);

    n->left->left = new Node(4);
    n->left->right = new Node(5);

    n->right->left = new Node(6);
    n->right->right = new Node(7);

    print(n);
    auto nc =  make_copy(n);
    print(nc);
}

Assignment operator.

The assignment operator is not 100% exception safe.
You should not modify the current object while you have not yet finished making the copy.

Node& Node::operator=(const Node& other)
{
    value = other.value;

    Node * left_orig = left;
    left = new Node(*other.left);
    delete left_orig;

    // You have modified the left side.
    // If while makeing the copy of the right side
    // it throws an exception (that is caught). Then you
    // have an object that is half one thing (new stuff)
    // and half the other (old stuff on the right).
    Node * right_orig = right;
    right = new Node(*other.right);
    delete right_orig;

    return *this;
}

It should look more like this:

Node& Node::operator=(const Node& other)
{
    Note* newleft  = nullptr;
    Node* newright = nullptr;

    // Do the dangerous stuff in isolation.
    // without changing the chaging the current object.
    try {
        newleft  = new Node(*other.left);
        newright = new Node(*other.right);
    }
    catch(...) {
        // If there was a problem
        // clean up any temporary objects.
        delete newleft;
        delete newright;
        // Then re-throw
        throw;
    }
    // Now perform the exception safe change of state.
    // None of these operations are allowed to throw.
    value = other.value;
    std::swap(left,  newLeft);
    std::swap(right, newRight);

    // Now that the object is in a consistent state.
    // we can delete the old data.
    // Do this last (in other types where the data is not in
    // this can potentially throw).
    delete newLeft;
    delete newRight;

    return *this;
}

Now that looks like a lot of hard work.
There is an easier way to achieve exactly the same affect. You can use the copy and swap idiom.

Node& Node::operator=(Node other)    // Pass by value to generate a copy.
{
    other.swap(*this);               // Swap the state of this and the
                                     // copy we created in `other`
    return *this;
}                                    // destructor of other now
                                     // does the tidy up.

Destructor

Calling delete on a nullptr is valid and does nothing.

Node::~Node()
{
    if (left != nullptr )
    {
        delete left;
    }
    if (right != nullptr)
    {
        delete right;
    }
}

So we can simplify this too.

Node::~Node()
{
    delete left;
    delete right;
}

Why do you need a make_copy?

Node* make_copy(Node* other)
{
    Node * new_node = new Node(*other); // copy constructor invoked
    return new_node;
}

It is quite normal to call the copy constructor directly.

Print statement

You can make the print function a method. Also to make it more versatile you should pass a stream to which you want to print (so it can also print to file). If you want a no argument version just make the stream parameter default to std::cout.

void Node::print(std::iostream& str = std::cin)
{
    std::cout << value << std::endl;
    if (left) {
        left->print(str);
    }
    if (right) {
        right->print(str);
    }
}

This makes it easy to define the output operator for your class.

std::ostream& operator<<(std::ostream& str, Node& data)
{
    data.print(str);
    return *this;
}


Markup "YYYY-MM-DD" as input hint I want to give users help in which format they have to enter a date. The label content is:

  Date (YYYY-MM-DD)

I used the following markup:

Date (YYYY-MM-DD)

Note: the new HTML5 input-type values (like date) might be used, but this label is a fallback for users with older browsers. Most of them use screenreaders and I try to give the most nitty-gritty markup.

I wonder:

Should I use the code element?
Is my use of the kbd element correct in this context? The "YYYY-MM-DD" represents something that users should enter, but it isn't something they should enter exactly as given, of course. It's more some kind of variable, I guess.
So maybe the var element might be appropiate?
Is my use of the abbr element correct?
Should I use the dfn element around each abbr element? So "YYYY" would be defined as year, because strictly spoken it is not an abbreviation for "four-digit year" but for "Year Year Year Year".

There is little point in using code markup here. Technically, it might not be completely wrong “semantically”, since the YYYY-MM-DD notation is really meant for computers rather than human beings, but what would it achieve? Monospace font by default?

Using kbd is equally, if not more, pointless.

Using var might be “semantically” appropriate, since you are using placeholders of a kind. But, here, too, the question is: Why? Do you want some browsers to render the text in italics? What else would you achieve in practice?

Using abbr is “semantically” wrong, since e.g. YYYY is not an abbreviation. It is a conventional notation. In practice, abbr creates by default a dotted bottom border on some borders. Why use some markup when the only real implication is that you get some default rendering that you don’t want?

Using dfn would be equally wrong. You are not giving any definition for e.g. YYYY. Instead, you are implying a definition, i.e. assuming that the user knows what YYYY means.

The title element is poor usability, but if you wish to use it, you can use it for a span element, which has no defined semantics (so it cannot be semantically wrong) and has no impact on default rendering.

If you don’t think that the explanation ”(YYYY-MM-DD)” is not sufficient, the best approach is to explain it in normal content. E.g.,

Date (YYYY-MM-DD, i.e. 4-digit year, hyphen, 2-digit month, hyphen, 2-digit day):

Note that a label element that has neither a for attribute nor a form field as content is pointless. The label markup is useful only when used to associate a form field with its label text.


Is a Timer the best way to achieve recursive in Android? I'm changing the Viewpager items programmatically using mViewPager.setCurrentItem(position); with a regular interval of time recursively. Please tell me if this is the best way to achieve this or if there are any problems with my code.

    ViewPager mViewPager ;
    mViewPager = (ViewPager) findViewById(R.id.viewpager);

    Timer timer = new Timer();
    timer.schedule(new UpdateTimeTask(backgroundimages.length), 1, 5000);

    private class UpdateTimeTask extends TimerTask {
        int mLength = 0;

        public UpdateTimeTask(final int length) {
            this.mLength = length;
        }

        public void run() {
            mViewPager.post(new Runnable() {
                @Override
                public void run() {
                    if (mViewPager.getCurrentItem()+1 < mLength) {
                        Updateviewpager(mViewPager.getCurrentItem() + 1);
                    } else {
                        Updateviewpager(0);
                    }
                }
            });
        }
    }

    public void Updateviewpager(int position) {
        mViewPager.setCurrentItem(position);
    }

Timers and TimerTasks appear to be discouraged for Android, especially when interacting with the UI. Each Timer gets a thread on which to do its work, which makes UI updates problematic, and which also presumably causes unnecessary overhead for small tasks.

This could be updated to use the postDelayed() or postAtTime() method of Handler, which takes advantage of the existing message queue.  Note: this code is not tested.

ViewPager mViewPager ;
mViewPager = (ViewPager) findViewById(R.id.viewpager);

private class UpdateTimeTask implements Runnable {
    private static final int TASK_TIME_MS = 5000;

    private int mLength = 0;
    private Handler mHandler;

    public UpdateTimeTask(final int length) {
        this.mLength = length;
        // Get the main thread's Handler. If the task is
        // always on the main thread, we can just do
        // mHandler = new Handler();
        // Otherwise, do:
        mHandler = new Handler(Looper.getMainLooper());

        // Get things started.
        // If this is intended to be done on the UI thread,
        // just call run() directly.
        // run();
        // If this is intended to be done on any thread, use
        // the Handler to execute run()
        mHandler.post(this);
    }

    public void run() {
        if (mViewPager.getCurrentItem()+1 < mLength) {
            Updateviewpager(mViewPager.getCurrentItem() + 1);
        } else {
            Updateviewpager(0);
        }

        // Schedule to repeat
        mHandler.postDelayed(this, TASK_TIME_MS);
    }
}

public void Updateviewpager(int position) {
    mViewPager.setCurrentItem(position);
}

References:
Android Timer Documentation
Updating the UI from a Timer


Verifying column and line counts I have created this simple code:

  For the size of an array with 2 values (n and m), verify if n and m are >= 1. m = width and n = height.

The code works but I'm sure it isn't optimized.

printf("Number of ligns.");
scanf("%d",n);
printf("Number of columns.");
scanf("%d",m);

if(*n >= 1)
{
    if(*m >= 1)
    {
        printf("Number of ligns : %x, Number of columns : %x \n", *n, *m);
    }
    else
    {
        while(*m < 1)
        {
            printf("Error, number of colums/ligns < 1. \n");
            scanf("%d",m);
        }
        printf("Number of ligns : %x, Number of columns : %x \n", *n, *m);
    }
}
else
{
    while(*n < 1)
    {
        printf("Error, number of colums/ligns < 1. \n");
        scanf("%d",n);
    }
            if(*m >= 1)
    {
        printf("Number of ligns : %x, Number of columns : %x \n", *n, *m);
    }
    else
    {
        while(*m < 1)
        {
            printf("Error, number of colums/ligns < 1. \n");
            scanf("%d",m);
        }
        printf("Number of ligns : %x, Number of columns : %x \n", *n, *m);
    }
}

You should provide a compiling code. I'll assume that n and m are int, but you should have added their declaration.

You should use explicit names for variables. n is a line count, so call it line_count. m is column_count.
You should check the return values of scanf to be sure you handle gracefully EOF and bad input.
You print "Error, number of colums/ligns < 1. \n" every time, so the user can't know if it's the lines or the columns or both that are < 1.
The english word is "lines", not "ligns".
Your checks are too complicated and redundants. The entire if/else block can be replaced by this:

while(*line_count < 1)
{
    printf("Error, number of lines < 1. \n");
    scanf("%d", line_count);
}

while(*column_count < 1)
{
    printf("Error, number of colums < 1. \n");
    scanf("%d", column_count);
}

printf("Number of lines : %x, Number of columns : %x \n", *line_count, *column_count);


Scraping website for book listings, checking if database contains relevant listings, and displaying result in a Django view This code works but is very slow.  I am scraping a website for urls for various books, and then checking to see if we have complete records of those books in our database.  After I process the URL to get the book info, I see if we have it in our database.  If we don't, I move on, that's fine.  If we do, I need to check that we have a matching BookTextVer type; that foreign keys to the book, and filters on an attribute from the URL.  If we have it, great; if not, I add it to my list of dictionaries to eventually pass to the django template.

def book_missing(request):
    data = []
    books = book.objects.all()
    sitemap = "www.sitemap.com"
    page = requests.get(sitemap)
    tree = objectify.fromstring(page.content)
    regex = re.compile("books-(\d+)([A-Za-z]+)(\d+)([A-Za-z]+)")
    for node in tree.getchildren():
        book_url = node.book.text
        # e.g. https://www.sitemap.com/fdsys/pkg/books-27hr200nf/content-detail.html
        m = regex.search(book_url)
        object_type_cd = m.groups()[1]  # hr
        book_num = m.groups()[2]  # 200
        book_type = m.groups()[3]  # nf
        try:
            book = books.get(book_num=book_num,
                             object_type_cd_id=object_type_cd.upper())
        except book.DoesNotExist:
            pass
        if (BookTextVer.objects.filter(
            book=book,
            book_text_ver_type__short_name=book_type.upper()).exists()):
            pass  # matching book type exists, no processing needed
        else:
            data.append({"book": object_type_cd.upper() + book_num,
                         "book_type": book_type.upper()})
    context = {
        'data': data,
    }
    return render(request, 'etl/reports/gpo-book-missing.html', context)

After I process the URL to get the book info, I see if we have it in our database. If we don't, I move on, that's fine.

I guess you're referring to this part of the code:

try:
    book = books.get(book_num=book_num,
                     object_type_cd_id=object_type_cd.upper())
except book.DoesNotExist:
    pass

This doesn't do what you described,
because after the pass,
the rest of the loop body is executed instead of moving on the next iteration.
Perhaps you meant to use continue there instead of pass.

Instead of having an empty if block and an else block here,
it would be better to flip the condition and have only an if block without an else:

if (BookTextVer.objects.filter(
    book=book,
    book_text_ver_type__short_name=book_type.upper()).exists()):
    pass  # matching book type exists, no processing needed
else:
    data.append({"book": object_type_cd.upper() + book_num,
                 "book_type": book_type.upper()})

The query on BookTextVer tries to match by short_name.
Is that field indexed in the database.
If not, the query will be inevitably slow,
and indexing should be a significant improvement.

The current implementation runs 1 or 2 queries for every node in tree.getchildren().
You might get better performance if you first collect all the book_num, book_type, object_type_cd values from the document,
and formulate a single database query or constant number of database queries.
That would be a major change,
and possibly you might need to forego Django's OR-mapping benefits,
but the reduced number of database queries could make a big difference.


Nonogram puzzle solution space I've written some code to brute-force enumerate the solution space of nonogram puzzles up to a certain size. It does a good job up to the first ~4 billion puzzles, but I run out of disk space to store any bigger collection of the solution space set.

Because of some incredibly interesting patterns within the encoded set of solvable instances, I'd appreciate my code being reviewed for correctness. This is an NP-complete problem, but shows self-similarity and self-affinity in the solution space with the current implementation. Please, help me shoot down this idea as quickly as possible -- I'd like to regain a productive life :)

Here's how the code is supposed to work. It outputs an XYZ point cloud of the nonogram solution space given a puzzle's maximum width, great for visualization.

The code generates all the possible permutations of an arbitrarily sized boolean image. Each permutation is a puzzle solution to at least one, if not many, boolean image input pairs. The input pairs are "projections" of the solution from each lattice direction, while having the same axis-constrained continuous runs of set bits. The only difference between solution and input is very subtle: the unset padding between contiguous runs is flexible along an axis.

Here's an example pairing of inputs and solution. Note that the pictured top-right solution may not be unique for the given input images, and the given input images don't necessarily construct only that solution. It's merely an example of the "nonogram property."

You'll notice in the code I have a peculiar encoding of the inputs and solutions as integers, essentially a traversal of the image's cells converted to a bitstring. This encoding is chosen as a visualization convenience, and I've attained similar patterns with different traversal orders. The main goal with the encoding is to reduce dimensionality for plotting with a one-to-one correspondence of images to integers, avoiding the problem of colliding identifiers.

I'm including a montage of the first four iterations as subsets of the solution space shadow. The full shadow is essentially a look-up table for an NP oracle, so seeing patterns here could have remarkable consequences. Arranged from left to right are tables scaled to a common 512x512 resolution -- 4, 256, 262144, and 4294967296 puzzles accounted for, respectively. Each black pixel represents an input pair with no solution, white says a solution exists.

from sys import argv
from itertools import product, chain, groupby
from functools import partial
from multiprocessing import Pool

def indices(width):
    for a in range(width):
        for b in range(a+1):
            yield (b, a)
        for b in reversed(range(a)):
            yield (a, b)

def encode(matrix, width):
    return sum(1<<i for i, bit
               in enumerate(matrix[i][j] for i, j in indices(width))
               if bit)

def count_runs(row):
    return [sum(group) for key, group in groupby(row) if key]

def flex(solution, width):
    counts = list(map(count_runs, solution))
    for matrix in product((False, True), repeat=width**2):
        candidate = list(zip(*[iter(matrix)]*width))
        if list(map(count_runs, candidate)) == counts:
            yield candidate

def nonogram_solutions(solution, width):
    xy = solution
    yx = list(zip(*solution))
    enc_sol = encode(solution, width)
    return [(encode(xy, width), encode(yx, width), enc_sol)
            for xy, yx in product(flex(xy, width), flex(yx, width))]

def main(width):
    pool = Pool()
    sol_matrices = (list(zip(*[iter(matrix)]*width)) for matrix
                    in product((False, True), repeat=width**2))
    nonograms = partial(nonogram_solutions, width=width)
    solutions = pool.imap_unordered(nonograms, sol_matrices, 1)
    pool.close()
    for xy, yx, solution in chain.from_iterable(solutions):
        print(solution, xy, yx)
    pool.join()

if __name__ == "__main__":
    main(int(argv[1]))

There are no docstrings. What do these functions do? Lack of docstrings make the code hard to review, because we don't know what the functions are supposed to do.
The call indices(n) generates the Cartesian product of range(n) × range(n) in a particular order. A natural question is, does the order matter, or could we use itertools.product instead:

itertools.product(range(width), repeat=2)

The post explains why you've chosen the order. But how is someone reading the code supposed to know that? There needs to be a comment.
encode could be simplified to avoid the if:

sum(bit<<i for i, bit in enumerate(...))

This code:

for matrix in product((False, True), repeat=width**2):
    candidate = list(zip(*[iter(matrix)]*width))

is effectively the same as:

sol_matrices = (list(zip(*[iter(matrix)]*width)) for matrix
                in product((False, True), repeat=width**2))

and so would benefit from having its own function (which would have a name and docstring that would explain what it does).
The algorithm takes every nonogram puzzle, and then compares the run counts with the run counts for every nonogram puzzle of the same size. If width is \$ n \$, there are \$ 2^{n^2} \$ nonogram puzzles, and it takes \$ Ω(n^2) \$ to compute the run counts for a single puzzle, so the overall runtime is the ludicrous \$ Ω(n^24^{n^2}) \$.

This could be brought down to \$ Ω(n^22^{n^2}) \$ if you prepared a dictionary mapping run counts to sets of encoded nonogram solutions, and down to \$ Ω(n2^{n^2}) \$ if you prepared the list of all rows, prepared a dictionary mapping each row to its run counts, and then generated the puzzles and their run counts simultaneously using the Cartesian product of the rows.


Mutual recursion - Naming the opponent serve - pingpong Problem statement
Ping-pong

The ping-pong sequence counts up starting from 1 and is always either counting up or counting down. At element k, the direction switches if k is a multiple of 7 or contains the digit 7. The first 30 elements of the ping-pong sequence are listed below, with direction swaps marked using brackets at the 7th, 14th, 17th, 21st, 27th, and 28th elements:

Implement a function pingpong that returns the nth element of the ping-pong sequence without using any assignment statements.

You may use the function num_sevens, which you defined in the previous question.

Use recursion - the tests will fail if you use any assignment statements.

Hint: If you're stuck, first try implementing pingpong using assignment statements and a while statement. Then, to convert this into a recursive solution, write a helper function that has a parameter for each variable that changes values in the body of the while loop.

def pingpong(n):
    """Return the nth element of the ping-pong sequence.

    >>> pingpong(7)
    7
    >>> pingpong(8)
    6
    >>> pingpong(15)
    1
    >>> pingpong(21)
    -1
    >>> pingpong(22)
    0
    >>> pingpong(30)
    6
    >>> pingpong(68)
    2
    >>> pingpong(69)
    1
    >>> pingpong(70)
    0
    >>> pingpong(71)
    1
    >>> pingpong(72)
    0
    >>> pingpong(100)
    2
    >>> from construct_check import check
    >>> # ban assignment statements
    >>> check(HW_SOURCE_FILE, 'pingpong', ['Assign', 'AugAssign'])
    True
    """
    "*** YOUR CODE HERE ***"

Solution
package main

import "fmt"

func numSevens(x int) int {
    if x == 0 {
        return 0
    }
    if x%10 == 7 {
        return 1 + numSevens(x/10)
    } else {
        return numSevens(x / 10)
    }
}

func pingPong(n int) int {

    var player1 func(int, int, bool) int
    var player2 func(int, int, bool) int
    player1 = func(currentIndex int, pingPongValue int, receiveOpponentBall bool) int {
        if currentIndex == n {
            return pingPongValue
        }

        if receiveOpponentBall {
            return player1(currentIndex+1, pingPongValue+1, false)
        }

        if numSevens(currentIndex) > 0 || currentIndex%7 == 0 {
            return player2(currentIndex, pingPongValue, true)
        }
        return player1(currentIndex+1, pingPongValue+1, false)
    }

    player2 = func(currentIndex int, pingPongValue int, receiveOpponentBall bool) int {
        if currentIndex == n {
            return pingPongValue
        }

        if receiveOpponentBall {
            return player2(currentIndex+1, pingPongValue-1, false)
        }
        if numSevens(currentIndex) > 0 || currentIndex%7 == 0 {
            return player1(currentIndex, pingPongValue, true)
        }
        return player2(currentIndex+1, pingPongValue-1, false)
    }

    return player1(1, 1, false)
}

func main() {
    //fmt.Println(numSevens(12345))
    fmt.Println(pingPong(29))
}

Below code is written without using mutual recursion:
func pingPongRecur(n int) int {
    toggleValue := 1 ^ -1
    increment := 1

    var runSequence func(int, int) int
    runSequence = func(currentIndex int, pingPongValue int) int {
        if currentIndex == n {
            return pingPongValue
        }
        if numSevens(currentIndex) > 0 || currentIndex%7 == 0 {
            increment ^= toggleValue
        }
        return runSequence(currentIndex+1, pingPongValue+increment)

    }
    return runSequence(1, 1)
}

How to give better name than receiveOpponentBall variable? How to  improve other variable naming convention? based on given problem statement.

Please add improvements in the above code from aspect of readability & good abstraction

From the problem statement:

Implement a function pingpong that returns the nth element of the
ping-pong sequence without using any assignment statements.
You may use the function num_sevens, which you defined in the previous
question.
Use recursion - the tests will fail if you use any assignment
statements.
Hint: If you're stuck, first try implementing pingpong using
assignment statements and a while statement. Then, to convert this
into a recursive solution, write a helper function that has a
parameter for each variable that changes values in the body of the
while loop.

Code should be correct, maintainable, robust, reasonably efficient, and, most importantly, readable.

Your first solution makes far too much of a simple switch in the increment value with duplicated functions differing merely in the literal integer values 1 and -1. It requires a diff comparison to find the small, obscure differences between the largely duplicate player1 and player2 functions. Duplication of code hurts maintainability. The solution introduces an obscure player abstraction which is not part of the problem statement. It is not readable. Also, the solution uses multiple assignment statements for the player function variables: "the tests will fail if you use any assignment statements."

While the problem statement did say you may use num_sevens, this problem asks "if k contains the digit 7". To count all occurences of digit 7 when we only care about the first occurence is inefficient.

Your second solution, "code written without using mutual recursion", which you added later, is clearly incorrect. It uses multiple assignment statements: "the tests will fail if you use any assignment statements." For no apparent reason, you use the name toogle instead of switch and you complicate things by introducing a toggleValue variable.
toggleValue := 1 ^ -1
increment := 1
. . .
        increment ^= toggleValue

The toggleValue variable is unnecessary and the toggle algorithm is obscure. Simplicity is a virtue.
increment := 1
. . .
        increment *= -1 // switch

The Problem Statement is an academic problem designed to discover if you have learned some fundamental programming concepts: recursion and no assignment statements. The Hint gives a detailed road-map to a solution. You did not answer the problem as stated so you get an F.

Here is an example of the sort of solution that the problem statement likely expected:

Use recursion. First try implementing pingpong using assignment
statements and a while statement. Then, to convert this into a
recursive solution, write a helper function that has a parameter for
each variable that changes values in the body of the while loop.

package main

import "fmt"

func digitSeven(x int) bool {
    if x <= 0 {
        return false
    }
    if x%10 == 7 {
        return true
    }
    return digitSeven(x / 10)
}

func increment(incr int, x int) int {
    if x%7 == 0 || digitSeven(x) {
        return incr * -1
    }
    return incr
}

func pingpong(n int, i int, v int, incr int) int {
    if i > n {
        return v
    }
    return pingpong(n, i+1, v+incr, increment(incr, i))
}

func PingPong(n int) int {
    return pingpong(n, 1, 0, 1)
}

func main() {
    for n := 1; n <= 30; n++ {
        fmt.Println(n, PingPong(n))
    }
}

Playground: https://play.golang.org/p/8H6l-CpLlye
Output:
1 1
2 2
3 3
4 4
5 5
6 6
7 7
8 6
9 5
10 4
11 3
12 2
13 1
14 0
15 1
16 2
17 3
18 2
19 1
20 0
21 -1
22 0
23 1
24 2
25 3
26 4
27 5
28 4
29 5
30 6


A* in Clojure - trickier than I expected While working through some coding puzzles to brush up on Clojure, I encountered the need to use A* on some search problems.  I tried to implement it from first principles/memory and that didn't go well, so I looked for existing implementations and found two (Joy of Clojure and cgrand's) but they were a little difficult to follow (for me) so I attempted to create my own.  In the end it actually looks pretty close to cgrand's implementation.

I think this works as expected (using an example from here to verify), but just wanted to post to see if anybody can point out any glaring omissions / oversights.  I've implemented this before in Python but it turned out to be quite different without using mutation.

The only issue I see is that I generate neighbors before testing whether I've already seen the node, so there is some inefficiency there. I couldn't figure out a good way to avoid this since my neighbor generation / filtering occurs in one step during the recursive call, whereas in languages allowing mutability you would process each child separately and you can test if it exists in the visited set each time.

(defn a-star-search
 [start neighbor-func goal? remain-cost path-cost]
  (loop [q (conj (sorted-set) [0 start])
         cost-so-far {start 0}
         came-from   {start nil}]
    (if-let [[node-cost node :as node-state] (first q)]
      (if (goal? node)
          (reverse (take-while (complement nil?) (iterate came-from node)))
          (let [neighbors (neighbor-func node)
                prev-node (came-from node)
                prev-cost (cost-so-far node)
                cheaper (remove #(< (cost-so-far % Double/POSITIVE_INFINITY)
                                    (+ prev-cost (path-cost node %)))
                                neighbors)
                new-nodes (map #(vector (+ prev-cost
                                           (path-cost node %)
                                           (remain-cost %)) %)
                               cheaper)]
            (recur (into (disj q node-state) new-nodes)
                   (->> cheaper
                        (map #(vector % (+ prev-cost (path-cost node %))))
                        (into cost-so-far))
                   (into came-from (map (juxt identity (fn [_] node)) cheaper)))))
      "no more neigbors")))

(def graph {:s {:a 1 :b 4}
            :a {:b 2 :c 5 :g 12}
            :b {:c 2}
            :c {:g 3}})

(def h-vals {:s 7 :a 6 :b 2 :c 1 :g 0})

 (a-star-search :s
             #(-> % graph keys)
             #(= :g %)
             h-vals
             (fn [from to] (get-in graph [from to])))

I'm not going to be able to do a very thorough review. The code doesn't have anything outright wrong with it, and I'll admit, I've never been able to implement a decent A* implementation. It's just not something that's ever clicked for me very well (cue a new project to fix that).

That said, there's a few minor things that can be pointed out:

(fn [_] node) can be written as (constantly node). Your use of complement suggests you like to avoid explicit functions.

You can also replace:

(map (juxt identity (fn [_] node)) cheaper)

with:

(map vector cheaper (repeat node))

I personally find the latter to be much cleaner. (map vector xs ys) zips xs and ys together into pairs. Here, I'm just zipping cheaper with an infinite list of nodes.

#(-> % graph keys)

can be replaced with:

(comp keys graph)

If, again, you want to avoid explicit functions (although that's not necessarily better).

I don't think returning "no more neighbors" is a great idea. If you need an indicator value for something like this, I'd probably define at the top something like:

(def no-neighbors ::no-neighbors)

Then return no-neighbors. You could also just use ::no-neighbors inline wherever you need it, but then you need to change it multiple places if you ever decide to change the keyword used. IDE's can help with that, but I like to just avoid the problem.

You could avoid the efficiency problem regarding neighbors if you had neighbor-f return a lazy sequence of keys. I can't off hand think of a good way to do that though.

You never use prev-node. It's good to remove dead code like that to remove bulk.

I think that's all I can really comment on. Hopefully someone else can give you a better review regarding the algorithm itself.

Good luck!


Strand Sort in Python After reading about Strand Sort on Wikipedia and testing the provided Python version, I decided to try my hand at a faster implementation. I've studied the "Time Complexity" page, read many different approaches to mergesort (and borrowed some), tested the code myself with profiler, line_profiler, and timeit, and I think I've got sped up pretty well.

I'm looking for any ideas on how to optimize this code without changing the sorting algorithm itself or resorting to using Python's timsort for any number of elements.

def strand_sort(array):
    if len(array) < 2:
        return array
    result = []
    while array:
        i = 0
        sublist = []
        sublist.append(array.pop())
        while i < len(array):
            num = array[i]
            if num > sublist[-1]:
                sublist.append(num)
                del array[i]
            else:
                i = i + 1
        result = merge(list(result), sublist)
    return result

def merge(list_1, list_2):
    i = 0
    j = 0
    merged_list = []
    while i < len(list_1) and j < len(list_2):
        if list_1[i] > list_2[j]:
            merged_list.append(list_2[j])
            j += 1
        else:
            merged_list.append(list_1[i])
            i += 1
    merged_list += list_1[i:]
    merged_list += list_2[j:]
    return merged_list

You're wasting a bit of time instantiating an empty list to instantly append to it with sublist. You'd be better off creating sublist as a list containing the array.pop() element.

    sublist = [array.pop()]

You could also save time in merge by just calling len once at the start. Since these lists aren't modified in the loop there's no need to call it on each iteration.

Stylewise, you could have better names. I know you can't call it a list, but array isn't quite accurate. Maybe sortable or collection? More importantly, you don't need to use num when your algorithm actually works on characters lexicography.

>>> strand_sort("See this can actually still be sorted".split())
['See', 'actually', 'be', 'can', 'sorted', 'still', 'this']

This obviously causes a little oddness with capitals being sorted before other letters. But since you're just using the > symbol, then this algorithm works on any object that has a function for the operator. I think it's worth noting in a docstring that this can be done, and you can highlight its limitation for sorting strings or explicitly say not to use it for strings even though it's possible.


RSA algorithm implementation in Python 3 First, a reminder of the RSA algorithm and what my program implements:

Take two distinct, large primes p and q

Ideally these have a similar byte-length

Multiply p and q and store the result in n
Find the totient for n using the formula $$\varphi(n)=(p-1)(q-1)$$
Take an e coprime that is greater, than 1 and less than n
Find d using the formula $$d\cdot e\equiv1\mod\varphi(n)$$

At this point, the pair (e, n) is the public key and the private key (d, n) is the private key.

Conception:

Implement the RSA algorithm
Ask the user for necessary data (primes, coprime greater than 1 and less than n, string)
Encrypt and decrypt the given string by the user using the RSA algorithm

What do you think about my Python 3 implementation of the RSA algorithm? What's the performance of this program?

try:
   input = raw_input
except NameError:
   pass
try:
   chr = unichr
except NameError:
   pass
p=int(input('Enter prime p: '))
q=int(input('Enter prime q: '))
print("Choosen primes:\np=" + str(p) + ", q=" + str(q) + "\n")
n=p*q
print("n = p * q = " + str(n) + "\n")
phi=(p-1)*(q-1)
print("Euler's function (totient) [phi(n)]: " + str(phi) + "\n")
def gcd(a, b):
    while b != 0:
        c = a % b
        a = b
        b = c
    return a
def modinv(a, m):
    for x in range(1, m):
        if (a * x) % m == 1:
            return x
    return None
def coprimes(a):
    l = []
    for x in range(2, a):
        if gcd(a, x) == 1 and modinv(x,phi) != None:
            l.append(x)
    for x in l:
        if x == modinv(x,phi):
            l.remove(x)
    return l
print("Choose an e from a below coprimes array:\n")
print(str(coprimes(phi)) + "\n")
e=int(input())
d=modinv(e,phi)
print("\nYour public key is a pair of numbers (e=" + str(e) + ", n=" + str(n) + ").\n")
print("Your private key is a pair of numbers (d=" + str(d) + ", n=" + str(n) + ").\n")
def encrypt_block(m):
    c = modinv(m**e, n)
    if c == None: print('No modular multiplicative inverse for block ' + str(m) + '.')
    return c
def decrypt_block(c):
    m = modinv(c**d, n)
    if m == None: print('No modular multiplicative inverse for block ' + str(c) + '.')
    return m
def encrypt_string(s):
    return ''.join([chr(encrypt_block(ord(x))) for x in list(s)])
def decrypt_string(s):
    return ''.join([chr(decrypt_block(ord(x))) for x in list(s)])
s = input("Enter a message to encrypt: ")
print("\nPlain message: " + s + "\n")
enc = encrypt_string(s)
print("Encrypted message: " + enc + "\n")
dec = decrypt_string(enc)
print("Decrypted message: " + dec + "\n")

I think your Modular Inverse implementation is slow. We can apply this Extended GCD algorithm recursive implementation which shows quite a dramatic speed improvement (at least on my machine):

def egcd(a, b):
    if a == 0:
        return b, 0, 1
    else:
        g, y, x = egcd(b % a, a)
        return g, x - (b // a) * y, y

def modinv(a, m):
    g, x, y = egcd(a, m)
    if g != 1:
        raise Exception('modular inverse does not exist')
    else:
        return x % m

Here is a simple benchmark for some a and m which have a modular inverse of 16013:

In [24]: a = 108 ** 151

In [25]: m = 22499

In [26]: %timeit modinv_OP(a, m)
100 loops, best of 3: 11.2 ms per loop

In [27]: %timeit modinv_new(a, m)
100000 loops, best of 3: 5.62 µs per loop

I also wonder if adding an LRU cache via functools.lru_cache() would make an impact - probably not, but please experiment with that.

Some other improvements:

you can avoid converting s to a list in order to iterate character by character:

''.join(str(chr(encrypt_block(ord(x)))) for x in s)

I've also switched to a "lazy" join when joined from a generator.
if on Python 3.6+, you can use f-strings for your print messages


ways to code for better speed of processing date comparisions / slicing in python I wrote the below code to convert a long data frame containing start and end date event spans into a daily df with rows for each day:
output:
 **bene_id, day, death, hha**
 row 1: abc, 1 ('2018-10-01'), 0,0
 row 2: abc, 2 ('2018-10-02'), 0,1
 row 3: abc, 3 ('2018-10-03'), 0,0
 row 4: abc, 4 ('2018-10-04'), 0,1

I am planning to use the daily output in a Tableau viz.
The below code - which does work - heavily uses date comparisons and slicing - is very, very, very slow. Are there particular functions I am using that have faster alternatives? Both the for loop and the functions are slow..
from pandas import Timestamp, Series, date_range

#creates empty df for input with correct column order
long = pd.DataFrame(columns={'bene_id', 'day','date'})
cols_to_order = ['bene_id', 'day','date']
new_columns = cols_to_order + (long.columns.drop(cols_to_order).tolist())
long = long[new_columns]

#gets only necessary columns for processing from main data set
sample=s[['bene_id','event_type','event_thru_date','look_forward_90_date','service_epi_from_date','service_epi_thru_date']]

#creates the long daily table with count 1 to 90, and daily date freq
for e in sample.bene_id.drop_duplicates():
    temp=sample[sample['bene_id']==e]
    start =Timestamp(temp[temp['event_type'] =='trigger'][['event_thru_date']].iloc[0][0])
    stop= temp[temp['event_type'] =='trigger'][['look_forward_90_date']]+pd.DateOffset(1)
    stop=Timestamp(stop.iloc[0][0])
    for i,j in zip(range(1,91), Series(date_range(start,stop))):
        long = long.append(pd.Series([e,i,j],index=cols_to_order), ignore_index=True)

#create functions to add events to daily df created above "long"; count first day of event span but not last date.
def checkdate(row,event):
    temp=sample[(sample['bene_id']==row['bene_id'])&(sample['event_type']==event)]
    temp['flag']= np.where((temp['service_epi_from_date']<=row['date']) &(temp['service_epi_thru_date']>row['date']),1,0)
    daily_status =temp['flag'].sum()
    return daily_status

def checkdeath(row,event):
    temp=sample[(sample['bene_id']==row['bene_id'])&(sample['event_type']==event)]
    temp['flag']= np.where(temp['service_epi_from_date']<=row['date'],1,0)
    daily_status =temp['flag'].sum()
    return daily_status

#apply functions referencing events in original sample df
long['death']=long.apply(checkdeath, axis=1, args=('death',))
long['hha']=long.apply(checkdate, axis=1, args=('hha',))

There are a few small niceties that you can add.
long = pd.DataFrame(columns={'bene_id', 'day','date'})
cols_to_order = ['bene_id', 'day','date']

should reuse the list:
cols_to_order = ['bene_id', 'day','date']
long = pd.DataFrame(columns=set(cols_to_order))

This:
cols_to_order + (long.columns.drop(cols_to_order).tolist())

can drop the outer parens, since . takes precedence over +.
e, j and temp need better names.
This:
for i,j in zip(range(1,91), Series(date_range(start,stop))):
    long = long.append(pd.Series([e,i,j],index=cols_to_order), ignore_index=True)

shouldn't be a loop. There's almost certainly a way for Pandas to vectorize it.


Beginner Tic Tac Toe game in Python I've just started learning python and I tried making a tic tac toe game and would really appreciate it if I got some feedback. I know, my code isn't the best optimized but this is what I could come up with.

row1 = ['/','/','/']
row2 = ['/','/','/']
row3 = ['/','/','/']

def xsturn():
    row = input('X, which row?')
    place = input('X, which place?') - 1
    if row == 1:
        if row1[place] == '/':
            row1[place] = 'X'
        else:
            print 'Move not possible!'
            xsturn()
    elif row == 2:
        if row2[place] == '/':
            row2[place] = 'X'
        else:
            print 'Move not possible!'
            xsturn()
    elif row == 3:
        if row3[place] == '/':
            row3[place] = 'X'
        else:
            print 'Move not possible!'
            xsturn()

def osturn():
    row = input('O, which row?')
    place = input('O, which place?') - 1
    if row == 1:
        if row1[place] == '/':
            row1[place] = 'O'
        else:
            print 'Move not possible!'
            osturn()
    elif row == 2:
        if row2[place] == '/':
            row2[place] = 'O'
        else:
            print 'Move not possible!'
            osturn()
    elif row == 3:
        if row3[place] == '/':
            row3[place] = 'O'
        else:
            print 'Move not possible!'
            osturn()

def win():
    value = 0
    if row1[0] == row1[1] == row1[2]:
        if row1[0] != '/':
            value += 1
    elif row2[0] == row2[1] == row2[2]:
        if row2[0] != '/':
            value += 1
    elif row3[0] == row3[1] == row3[2]:
        if row3[0] != '/':
            value += 1
    elif row1[0] == row2[0] == row3[0]:
        if row1[0] != '/':
            value += 1
    elif row1[1] == row2[1] == row3[1]:
        if row1[1] != '/':
            value += 1
    elif row1[2] == row2[2] == row3[2]:
        if row1[2] != '/':
            value += 1
    elif row1[0] == row2[1] == row3[2]:
        if row1[0] != '/':
            value += 1
    elif row1[2] == row2[1] == row3[0]:
        if row1[2] != '/':
            value += 1
    return value

def printingrows():
    print row1
    print row2
    print row3

def game():
    while win() == 0:
        xsturn()
        printingrows()
        if win() == 1:
            print 'X won the game!'
            break
        else:
            osturn()
            printingrows()
            if win() == 1:
                print'O won the game!'
                break
game()

I ran your code, and played a bit:

$ python2 test.py
X, which row?1
X, which place?1
['X', '/', '/']
['/', '/', '/']
['/', '/', '/']
O, which row?2
O, which place?2
['X', '/', '/']
['/', 'O', '/']
['/', '/', '/']
X, which row?3
X, which place?1
['X', '/', '/']
['/', 'O', '/']
['X', '/', '/']
O, which row?2
O, which place?1
['X', '/', '/']
['O', 'O', '/']
['X', '/', '/']
X, which row?2
X, which place?4
Traceback (most recent call last):
  File "test.py", line 95, in <module>
    game()
  File "test.py", line 84, in game
    xsturn()
  File "test.py", line 15, in xsturn
    if row2[place] == '/':
IndexError: list index out of range

First, there is no presentation of the board when play starts. So you're relying on the X player to know what's going on.

Second, using two lines to ask for the move raises the issue of making a mistake and needing to go back. I'd suggest you either add a "go back" feature of some kind, or convert to using a single line of input for the entire move- some kind of X, Y input, or numbering the cells from 1-9.

Third, you don't do any range checking on the input, as is obvious from the end of my game. Given that you don't prompt the user with a definition of what input is valid (1-3), that's a double error. You need to tell the user what's valid, and double-check the input to make sure that it is, in fact, valid.

Fourth, you need to improve the presentation of the board. You should display it prior to each move, even the first move, so the players know what's what. You should also label the board, so that input is obvious. If you ask what row, column for a move, then label the rows/columns on the board display. And please change the '/' into something more sensible - use a space (' ') or a dot ('.') or something that looks like an empty cell.

That said, let's look at some code:

def xsturn():
    row = input('X, which row?')
    place = input('X, which place?') - 1
    if row == 1:
        if row1[place] == '/':
            row1[place] = 'X'
        else:
            print 'Move not possible!'
            xsturn()
    elif row == 2:
        if row2[place] == '/':
            row2[place] = 'X'
        else:
            print 'Move not possible!'
            xsturn()
    elif row == 3:
        if row3[place] == '/':
            row3[place] = 'X'
        else:
            print 'Move not possible!'
            xsturn()

def osturn():
    row = input('O, which row?')
    place = input('O, which place?') - 1
    if row == 1:
        if row1[place] == '/':
            row1[place] = 'O'
        else:
            print 'Move not possible!'
            osturn()
    elif row == 2:
        if row2[place] == '/':
            row2[place] = 'O'
        else:
            print 'Move not possible!'
            osturn()
    elif row == 3:
        if row3[place] == '/':
            row3[place] = 'O'
        else:
            print 'Move not possible!'
            osturn()

These two functions are all but identical. Replace them with a single function taking a character parameter:

def take_turn(sigil):
    row = input(sigil + ', which row?')
    place = input(sigil + ', which place?') - 1
    if row == 1:
        if row1[place] == '/':
            row1[place] = sigil
        else:
            print 'Move not possible!'
            take_turn(sigil)
    # etc.

You can then modify the game function to pass in the player sigil:

def game():
    while win() == 0:
        take_turn('X')
        printingrows()
        if win() == 1:
            print 'X won the game!'
            break
        else:
            take_turn('O')
            printingrows()
            if win() == 1:
                print'O won the game!'
                break

Next, rename printingrows to something more accurate: show_board or some such. Try to always write your code using the vocabulary of the problem domain. In this case, the problem domain is a board game.

Finally, there are a bunch of opportunities for you to tighten things up. For example, checking for a win in the turn function instead of separately, or using a list-of-lists instead of three separate row lists. If you can make the changes above and update your code, then we can get into those changes.


Better paradigms for posting JSON from a table? With a table based interface where users can update the rows by pressing a button. Perhaps there is a better way of doing this, without pressing a button, i.e. post on textarea/row change (could be a selectbox column). My requirement is that it's the LEAST amount of JavaScript possible, without being cryptic.

For example, could the code here be improved (assume an uptodate Firefox/Chrome browser), ideally without changing the HTML?

$(document).ready(function () {
$('button').click(function () {
    var row = $(this).parent().parent();
    row.css("background-color", "red");
    var id = row.children(':nth-child(1)').text();
    var note = row.find('textarea').val();
    var data = {
        json: JSON.stringify({
            id: id,
            note: note
        })
    };

    $.ajax({
        url: "/echo/json/",
        type: "POST",
        data: data,
        contentType: "application/json"
    }).done(function (d) {
        console.log(d);
        row.css("background-color", "green");
    });
});
});

I want less code for the JavaScript and I was thinking how much effort if would be to remove the jQuery dependency entirely. Ashamedly it took me a long time to write that jQuery!

$('button').click(function () {
    var row = $(this).parent().parent();

The button selector is not specific enough. To make sure that .parent().parent() will be what you think it is, and to clarify it in the code, I would recommend this way instead:

$('tr > td > button').click(function () {

Having the id value as the text of a td field is not great. Judging by the name "id", it sounds like technical information that shouldn't really be visible. In HTML5 you can use attributes prefixed with data- for embedding technical details invisibly inside your HTML. So I would write your rows this way:

    Goonies

        <textarea></textarea>

        <button>Save</button>

And then you could get id like this:

var id = row.attr('data-id');

  Perhaps there is a better way of doing this, without pressing a button, i.e. post on textarea/row change (could be a selectbox column).

You could trigger your function on a change to the textarea, in addition to the button, for example:

function onRowChanged() {
    var row = $(this).parent().parent();
    row.css("background-color", "red");
    var id = row.attr('data-id');
    var note = row.find('textarea').val();
    var data = {
        json: JSON.stringify({
            id: id,
            note: note
        })
    };

    // ...
}
$('tr > td > button').click(onRowChanged);
$('tr > td > textarea').change(onRowChanged);

  I want less code for the JavaScript and I was thinking how much effort if would be to remove the jQuery dependency entirely.

For the record, I don't think there's anything wrong with having some JavaScript code. On the other hand, getting rid of jQuery can be a good thing, if you only need a tiny subset of its power. It's certainly possible to rewrite this small piece of code without jQuery, but I'm not very good at that myself. Instead, I can recommend zepto.js as an ultra-light jQuery replacement. You can try it in your fiddle, just change the Framework to Zepto, and make a small modification to your AJAX call:

$.ajax({
    url: "/echo/json/",
    type: "POST",
    data: data,
    contentType: "application/json",
    complete: function (d) {
        console.log(d);
        row.css("background-color", "green");
    }
});

I hope this helps, but you might want to wait for some real JavaScript experts to respond!


Prime less than N that is a sum of the most consecutive other primes Here's a Python implementation of the Sieve of Eratosthenes:

def return_primes(upto=100):
    primes = []
    sieve = set()
    for i in range(2, upto+1):
        if i not in sieve:
            primes.append(i)
            sieve.update(range(i, upto+1, i))
    return primes

I use the above function in the below algorithm to find the prime that is the sum of the most consecutive primes. I'm sure it has room for improvement. It starts with a small sliding window which iteratively increases in size, and each iteration goes until the found sum is greater than the largest prime, and then breaks to the outer iteration. Optimized for Python 2:

def find_prime_sum_of_most_consecutive_primes_under(x):
    '''
    given x, an integer, return the prime that is the sum
    of the greatest number of consecutive primes under x
    '''
    primes = return_primes(x)
    length = len(primes)
    primeset = set(primes)
    maxprime = primes[-1] # last prime
    result = None
    for window in xrange(1, length): # xrange should be range in Python 3
        iters = length - window
        for it in xrange(iters):
            s = sum(primes[it:window+it])
            if s in primeset:
                result = s
            if s > maxprime:
                break
    return result

Can this be improved?

Multiples of 2 cannot be prime and primes greater than 2 are odd. This means that the sum to be found must be the sum of an odd number of primes if 2 is not in the window. This lets you skip half the work.

You can update a sliding sum by subtracting the outgoing number and adding the incoming one. Perhaps this could be the basis of a window that moves forward as it expands and contracts, much like a caterpillar moves... The idea is to avoid redoing lots of sums without having to store lots of intermediary results. The basic sliding sum should already save quite a bit of work, though.

This can give you a quick exploratory run with sequences (sums) of maximum possible length, if you expand two sums from 2 and 5 respectively - skipping every other prime - and start sliding once the sums exceed the maximum prime. The disadvantage is that you do not gain information for bounding future searches, as in the bottom-up case where you never have to consider sequences shorter than the longest you've found so far. Perhaps it is better to caterpillar right from the start, to quickly collect growing lower bounds on sequence length.

Growing pair of snakes: slide two sums forward, offset by one prime, that are one double step longer than the longest sequence found so far. Expand as you find longer and longer sequences. That should give you some decent results in one stereo slither through the range, which can help bound future searches. (hindsight: one snake, single steps; it's only the growing that can be double-stepped)

UPDATE

I did a bit of snake charming and it didn't pan out - snakes grow very slowly and they have to move around a lot in order to do so. By and large there is little that can be done in the way of improving/refining bounds for the search, apart from using the length of the best sequence found so far (plus two) as a lower bound for future searches.

A lot of re-computation can be avoided by computing cumulative sums for the primes and storing them in a table; that way the sum from prime i to prime k is available as p_sums[k] - p_sums[i - 1] for i greater than zero.

Also, searches go faster if all possible sequence lengths are tried for a given starting point before moving to the next starting point, rather than fixing the sequence length for a given round and then trying all starting points. However, I haven't studied this point thoroughly; the success of this strategy may depend on the fact that a lot of winning sequences happen to begin at very low primes for the small range I tested (232).

'Brute force with superior firepower' is several orders of magnitude faster than the plain brute-force algorithm:

*** g++ 4.8.1 64 ***

v0:          10       0.000 ms          5 = {2} @ 2
v1:          10       0.000 ms          5 = {2} @ 2
v0:         100       0.001 ms         41 = {6} @ 2
v1:         100       0.000 ms         41 = {6} @ 2
v0:        1000       0.010 ms        953 = {21} @ 7
v1:        1000       0.001 ms        953 = {21} @ 7
v0:       10000       0.122 ms       9521 = {65} @ 3
v1:       10000       0.002 ms       9521 = {65} @ 3
v0:      100000       1.505 ms      92951 = {183} @ 3
v1:      100000       0.013 ms      92951 = {183} @ 3
v0:     1000000      19.793 ms     997651 = {543} @ 7
v1:     1000000       0.104 ms     997651 = {543} @ 7
v0:    10000000     254.493 ms    9951191 = {1587} @ 5
v1:    10000000       0.782 ms    9951191 = {1587} @ 5
v0:   100000000    4485.791 ms   99819619 = {4685} @ 7
v1:   100000000       6.226 ms   99819619 = {4685} @ 7
v0:  1000000000   68065.223 ms  999715711 = {13935} @ 11
v1:  1000000000      52.905 ms  999715711 = {13935} @ 11
v0: 10000000000 1359343.949 ms 9999419621 = {41708} @ 2
v1: 10000000000     485.861 ms 9999419621 = {41708} @ 2

The last round used more than 7 GB of core for the two tables, which means this is pretty much the end of the line for this simple algorithm even though the actual search took only half a second. The time for sieving the primes - about one second per 2^31 numbers for my simple code - is not included.

I'm showing the routine as C++ since my Python-fu is not sufficient for doing either python or the algorithm justice. At least that way you have an accurate description for the algorithm, and can ponder about devious ways of having Python do all the hard work:

typedef uint32_t num_t;
typedef uint64_t sum_t;

std::vector<num_t> primes;
std::vector<sum_t> p_sums;    // sum(primes[0] .. primes[i]) for i < primes.size()

void prepare_prime_tables_for_limit (num_t n);
bool is_prime (num_t n);      // uses std::lower_bound() on primes[], i.e. binary search

num_t brute_force_v1 (unsigned n = 1000000000)
{
   assert( n >= 10 );

   prepare_prime_tables_for_limit(n);

   num_t prime_count = num_t(primes.size());
   num_t max_prime = primes.back();
   num_t best_len = 1;
   num_t best_idx = 0;
   sum_t best_sum = 0;

   // separate pass for the prime that is so odd because it's even

   for (num_t i = 1; ; i += 2)
   {
      sum_t sum = p_sums[i];

      if (sum <= max_prime)
      {
         if (is_prime(num_t(sum)))
         {
            best_len = i + 1;
            best_sum = sum;
         }
      }
      else break;
   }

   --best_len;  // adjust for the evenness of 2

   for (num_t i = 1; i + best_len + 1 < prime_count; ++i)
   {
      for (num_t k = i + best_len + 1; ; k += 2)
      {
         sum_t sum = p_sums[k] - p_sums[i - 1];

         if (sum <= max_prime)
         {
            if (is_prime(num_t(sum)))
            {
               best_len = k - i + 1;
               best_idx = i;
               best_sum = sum;
            }
         }
         else break;
      }
   }

   best_len += best_idx == 0;  // restore the stolen bit if 2 is still best

   // ... print stuff ...

   return num_t(best_sum);
}

I left index math etc. unoptimised in order to make the code easier to understand. For the same reasons I didn't harden it by adding the complications needed for handling boundary cases (n < 8 etc.). My intent was to make the basic algorithm as clear as possible.

Sums need a bigger type than indexes and numbers, hence the occasional cast to num_t when it is safe (and necessary) to do so.


Filling in the card expiry date from transaction data I'm integrating my app with a payment gateway, and have the following code which returns an array to populate the transaction info:

function populate_transaction_info($data)
{
  extract($this->extract_model_data($data));

  // set to empty array to ensure run-able code
  $transaction_info = [];

  return array_merge($transaction_info, [
    'pan' => $card_data['card_number'],
    'expdate' => $this->build_expiry_date_string($card_data['card_exp']),
    'cust_id' => $person['email'],
    // ... more params here, not relevant for question
  ]);
}

// shown for context / run-ability
function extract_model_data($data)
{
  return [
    'person' => $data['Person'],
    'card_data' => $data['CreditCard']],
  ];
}

My question pertains to the build_expiry_date_string(), defined as:

function build_expiry_date_string($card_expiry)
{
  return $card_expiry['month'] . substr($card_expiry['year'], -2);
}

I'm passing $card_data['card_exp'] into the function, but I'm wondering if the function itself should be the only thing that knows that it needs to look in the card_exp index of the array? So for example, should I refactor as:

...
'expdate' => $this->build_expiry_date_string($card_data),
...

function build_expiry_date_string($card_data)
{
  return $card_data['card_exp']['month'] . substr($card_data['card_exp']['year'], -2);
}

If yes, why?

Your first version of build_expiry_date_string() will be able to build an shorter notation date from any date array. I would therefore rename it to build_short_date_string().

The second version actually is named correctly, but can only convert an expiry date to the shorter notation.

Which is the better version?

Well, clearly, if you need to create a short date string from more than one date array then the first one wins. The second one is more specialized. It will only work for the card_exp date.

In general I don't see the point of overspecializing functions. The more general purpose version does a good job, so I would prefer that one, even if it is, for now, only used for one date array.

Functions should be used to encapsulate functionality that can be reused and therefore make your code easier to maintain. (reference)

It is the reusability that wins for me here, over any other considerations.

Note that we're talking about simple functions here, not methods that are part of classes. OOP is a whole other can of worms.

Some other remarks about your code (this is code review afterall):

You extract() data at the beginning of populate_transaction_info(). This seems like a nice trick, but it makes your code harder to read and debug. For instance, I have to assume that $person is one of these extracted variables, but for me it appears out of thin air. You could help your code reader by using a prefix whenever your extract something: extract(...., EXTR_PREFIX_ALL, 'model'); and then use $model_person, but preferably just explicitly define the person variable in the function.
Your comment: "set to empty array to ensure run-able code" and the subsequent array_merge() make no sense to me. Why not just define the array?


Create a Promise that resolves when jQuery loads I'm currently experimenting with async loading of JavaScript and came up with the following trick to detect when jQuery loads:

var jQueryPromise = new Promise(function(resolve) {
  Object.defineProperty(window, '$', {
    get: function() {
      return this.jQuery;
    },
    set: function(value) {
      if (typeof value === 'function') {
        resolve(value);
      }
    }
  });
  if (typeof jQuery === 'function') {
    $ = jQuery;
  }
});

jQueryPromise.then(function($) {
    // use $ normally here
});

The idea is to have a method which works regardless of how and when jQuery was loaded (via a script tag, async script tag, script created via document.createElement...)

My aim is really just to understand how to best utilise async loading in general - all comments are welcome, especially regarding possible quirks of this (hacky) approach.

In addition, I'm curious whether there are any performance advantages to loading jQuery async and then waiting for the Promise to resolve using this method over just loading it normally before the script (and in which cases would this performance boost happen)?

This is... hacky. It's extra code that could have just gone away if you just loaded your scripts through proper methods.

If jQuery is ubiquitous, it might be more beneficial to just load it up front.
If you have a build step, then compile it as a dependency of your app.
If you're doing asynchronous modules, consider using RequireJS.

In addition, browser cache will do its part to reduce loading times. Only the initial load will be slow. Also, async does not help in any way with load speed. It only reduces the time your script will block the HTML.