Average of properties excluding some I have an array of objects with, let's say, two properties. One of those properties can have bad values, in the form on a -9999 value.

The aim is to calculate the average of each property, without the bad values.

I am looking to improve my code,  I feel especially wrong about the two loops.

var array = [
{item1: 50, item2: 10},
{item1: 70, item2: -9999},
{item1: 60, item2: 5},
{item1: 40, item2: 7},
]

var avg1 = array.reduce(function(sum, a,i,ar) { sum += a.item1;  return i==ar.length-1?(ar.length==0?0:sum/ar.length):sum},0);

document.getElementById("avg1").innerText = avg1;

var badvalues = 0;

for (var i=0; i<array.length; i++){
if (array[i].item2 == -9999){badvalues++;}
}

var avg2 = array.reduce(function(sum, a,i,ar) {
sum += (a.item2 == -9999? 0: a.item2);
return i==ar.length-1?(ar.length==0?0:sum/(ar.length-badvalues)):sum},0);

document.getElementById("avg2").innerText = avg2;

There is already a JavaScript function to aggregate values in an array: array.reduce(). You can use it like this:

const invalidValueForItem2 = -9999;

let result = {
    item1: { sum: 0, count: 0},
    item2: { sum: 0, count: 0},
};

result = array.reduce(function(acc, item) {
    acc.item1.sum += item.Item1;
    ++acc.item1.count;

    if (item.Item2 != invalidValueForItem2) {
        acc.item2.sum += item.Item2;
        ++acc.item2.count;
    }
}, result);

Using an object to hold your temporary data we may scale better when you will need more properties and/or if you will average all properties without any knowledge about them. Don't you need to average both Item1 and Item2? Just drop the outer object and keep { sum: 0, count: 0}:

let result = { sum: 0, count: 0};

result = array.reduce(function(acc, item) {
    if (item.Item2 != invalidValueForItem2) {
        acc.sum += item.Item2;
        ++acc.count;
    }
}, result);

Now you have your result you can calculate the average:

const average2 = result.item2.count > 0 ? (result.item2.sum / result.item2.count) : NaN;

Do not forget to handle the corner case where there aren't items to sum, in this example when there aren't items the average is NaN (but you might want 0), if Infinity is appropriate then just make the division.

Few more notes about your code:

Indentation is incredibly important. Write just few hundred lines without proper indentation and you won't be able to understand what your code is doing without re-reading everything (and probably introducing bugs here and there).

Unless you have to support older browser you'd better use let instead of var (and const where appropriate).

Let your code breath, add space (both horizontally and vertically) and it will be much easier to read. To make code unreadable but small is the job of Minifiers/Uglifiers! Compare this with your original call to reduce():

var avg1 = array.reduce(function(sum, a, i, ar) {
    sum += a.item1;
    return i == ar.length - 1 ? (ar.length == 0 ? 0 : sum / ar.length) : sum;
}, 0);

Those lines let me introduce few more things:

Do not nest ternary operators unless you're dealing with incredibly simple expressions.

You do not need all the parameters of the reduce() function (ar is just array) then it might be function(sum, a). Also you'd better find a meaningful name for a (especially in this case because function documentation talks about an accunulator then it may be misleading if you do not work often with it). Removing i will also stop you to do two things in this function (accumulate the sum and then return the average for the last one).

If your accumulator is a number (initialized to zero) then you do not need to explicitly give it as parameter.

How can you rewrite this? Just few lines when using an arrow function:

let sum = array.reduce((acc, item) => acc + item.item1);
let average = sum / array.length;

However you won't need this code because average for Item1 is calculated together with Item2, use this code for simpler cases.


Allow certain IP addresses to run a C++ program I coded an executable program (.exe) that I only want run either from my home computer, our main server, or people in our development team.

I have coded logic that will only allow the program to be run from certain IP addresses.

The .txt file that's referenced in the getIPlist function looks like this:

[123.456.78.90] = Main Server
[77.34.555.392] = My computer at home
[333.455.3.3] = Assistant coder

And the HTTP address that the parseAllowableIPs function points to is a PHP file that is simply coded:

<?php echo $_SERVER['REMOTE_ADDR']; ?>

This code is working great (except for a bit of memory not being freed), so a review or some efficiency tips are welcome.

bool compareIPs(std::string IPlist, std::string userIP)
{
    const char *U = userIP.c_str();
    char str[20];

    const char *goodList[25] = { '\0' };

    std::string uIP = "";

    while (*U)
    {
        if (atoi(U))
        {
            uIP += itoa(atoi(U), str, 10);
            while (*U && *U != '.')
                *U++;

            if (*U && *U == '.')
                uIP += ".";
        }

        if (*U)
            *U++;
    }

    const char *L = IPlist.c_str();
    int count = 0;
    std::string thisIP = "";

    while (*L)
    {
        switch (*L)
        {
        case '[':
            *L++;

            while (*L && *L != ']')
            {
                thisIP += *L;
                *L++;
            }

            goodList[count] = _strdup(thisIP.c_str());
            thisIP = "";
            count++;
            break;
        default:
            break;
        }

        if (*L && *L != '[')
            *L++;
    }

    std::string comp = "";

    // Now check to see if the user's IP matches any in the goodList[]
    for (int a = count; a >= 0; a--)
    {
        if (!goodList[a])
            continue;

        comp = goodList[a];

        if (!uIP.compare(comp))
        {
            // I need to free() what was _strdup()'d
            // but this causes the progam to crash.

            //free(&goodList[a]);

            return true;
        }
    }

    return false;
}

#pragma comment(lib, "WinInet.Lib")

std::string getIPlist()
{
    HINTERNET hInternet, hFile;
    DWORD rSize;
    char buffer[1024];

    hInternet = InternetOpen(NULL, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, 0);
    hFile = InternetOpenUrl(hInternet, "http://www.myWebServer.com/ip/authorizedIPlist.txt", NULL, 0, INTERNET_FLAG_RELOAD, 0);
    InternetReadFile(hFile, &buffer, sizeof(buffer), &rSize);
    buffer[rSize] = '\0';

    InternetCloseHandle(hFile);
    InternetCloseHandle(hInternet);

    std::string result = buffer;

    return result;
}

bool parseAllowableIPs()
{
    // Grap the list of allowable IP's and store it in a string
    // to be parsed in compareIPs() function.
    std::string allowableIPlist = getIPlist();

    HINTERNET hInternet, hFile;
    DWORD rSize;
    char buffer[1024];

    hInternet = InternetOpen(NULL, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, 0);
    hFile = InternetOpenUrl(hInternet, "http://www.myWebServer.com/ip/index.php", NULL, 0, INTERNET_FLAG_RELOAD, 0);
    InternetReadFile(hFile, &buffer, sizeof(buffer), &rSize);
    buffer[rSize] = '\0';

    InternetCloseHandle(hFile);
    InternetCloseHandle(hInternet);

    std::string result = buffer;

    if (!compareIPs(allowableIPlist, result))
        return false;

    return true;
}

int main()
{
    //... blah blah blah

    // If not an authorized IP, just exit the game.
    if (!parseAllowableIPs())
    {
        MySQL__disconnect();
        exit(588924);
    }
}

It took me a while to figure out what the code was trying to do.  I guess that http://www.myWebServer.com/ip/index.php serves a purpose similar to http://www.whatismyip.com, echoing the HTTP client's IP address.  That is better than just enumerating the IP addresses on the client machine's interfaces, as it ensures that you pick an address that is actually routable.

Like all DRM schemes, this one is crackable too.  One simple workaround is to use a transparent HTTP proxy that intercepts the call-home message and fakes a response.  Using HTTPS with server certificate validation would help prevent that man-in-the-middle attack.

The parseAllowableIPs() function is misnamed.  It's not just parsing — it's also fetching, reading, and making an allow/deny decision.  A more appropriate name would be isClientIPAuthorized().

compareIPs() is doing more than its fair share.  Instead of returning a std::string, getIPlist() should also split the text into a std::vector<std::string>.  There should also be a getMyIP() function.  Then, compareIPs() would be a simple verification of whether an item is in a list.

In summary, the outline should be more like:

bool isClientIPAuthorized()
{
    std::string me = getMyIP();
    std::vector<std::string> allowed = getAllowableIPs();
    return std::find(allowed.begin(), allowed.end(), me) != allowed.end();
}

int main()
{
    //... blah blah blah

    // If not an authorized IP, just exit the game.
    if (!isClientIPAuthorized())
    {
        MySQL__disconnect();
        exit(588924);
    }
}

However, I'd go a step further.  The dotted-quad parsing code in compareIPs() has some minor issues, such as superfluous pointer dereferencing.  (Note that it would be better to write U++; rather than *U++;, even though the effect is the same.)  It would be better to avoid reinventing the wheel, and using a standard IP address parsing routine.  (It's such a common task, a few minutes with Google and MSDN is worth it.)  There's the Winsock function inet_aton(), which parses a IPv4 dotted-quad address into a long, and the newer InetPton() that also handle IPv6.  So, getMyIP() and getAllowableIPs() should probably return long and std::vector<long>, respectively, instead of strings.


Better way to write 'bulky' function in jQuery I have this function that works fine, but it looks really ugly and repetitive...

Basically I have a featured profile and underneath I have 4 smaller thumbnails, on click I want to populate the large profile. The code works fine it just doesn't look too good...

$('ul.teamProfiles li').click(function () {

    var $name = $(this).find('h4.name').html();
    var $picture = $(this).find('img').data('bigimg');
    var $biography = $(this).find('.biographyParas').html();
    var $jobTitle = $(this).find('p.jobTitle').html();
    var $telephone = $(this).find('p.telephone').html();
    var $linkedin = $(this).find('a.linkedinLink').attr('href');
    var $twitter = $(this).find('a.twitterLink').attr('href');
    var $mailto = $(this).find('a.mailtoLink').attr('href');

    $('.profileDetail .largeImg').attr('src', $picture);
    $('.profileDetail h3.name').html($name);
    $('.profileDetail h4.jobtitle').html($jobTitle);
    $('.profileDetail .biographyContainer').html($biography);
    $('.profileDetail span.telephone').html($telephone);
    $('.profileDetail a.linkedin').attr('href', $linkedin);
    $('.profileDetail a.twitter').attr('href', $twitter);
    $('.profileDetail a.email').attr('href', $mailto);
});

It's difficult to really clean it up, since a lot of it is "manual" translation from one piece of markup to another. Different elements, different attributes etc.

However, there's the low-hanging fruit (and golden rule): Don't repeat yourself.

So don't call $(this) over and over. It's just inefficient. Do something like

var source = $(this),
    target = $(".profileDetail");

and then chain off of those, like source.find('h4.name')...

However, a nicer way to do this might be to use a data-source/data-target attributes on the "big" profile elements. For instance

  ...

So in each element data-source is a comma-separated list of the selector to run in the source element, the (optional, defaults to html()) method to call on the result, and the (optional) arguments. Meanwhile, data-target is can be an attribute name, but it's optional and defaults to html()

Then, in the JS, do something like this:

$('ul.teamProfiles li').click(function (event) {
  var selection = $(this),
      destination = $(".profileDetail:first");

  // find and loop through elements with a data-source attribute
  destination.find("[data-source]").each(function () {
    var target = $(this),             // an element in the "big" profile container
        data = (target.data('source') || "").split(/\s*,\s*/), // parse the data-source attribute
        selector = data[0],           // get the selector
        method = data[1] || 'html',   // get the method (defaults to html)
        args = data.slice(2),         // slice off the arguments
        attr = target.data('target'), // get the destination attribute (if any)
        source, content;

    if(!selector) return; // nothing to do
    source = selection.find(selector); // find the source element in the small profile
    content = source[method].apply(source.get(0), args); // call the method with the args
    if(attr) {
      target.attr(attr, content); // set the target's attribute, if that's what's needed
    } else {
      target.html(content); // otherwise, default to inserting the content as html
    }
  });
});

That function is pretty generic (though it won't work for more complex stuff like setting a bunch of css).

The point is, that the mapping of small profile to large profile can be declared entirely in the HTML; the JS doesn't need to change if you add/remove elements.

Haven't tested it, but it should work


Pattern Matching with Mismatch I have been using a slightly modified Hamming Distance algorithm for approximate String Matching for patterns and wondering if there is something better out there. The t being the length of the text and p being the length of the pattern the worst case is roughly O(t*p). Which from looking at other Fuzzy String matching seems to be in norm.

    final int mismatches = 1;
    final String text = "bubbles";
    final String pattern = "bu";

    for(int iter = 0; iter < text.length() - pattern.length() + 1; iter++)
    {
        int missed = 0;
        int ator = 0;

        do
        {
            if(text.charAt(iter + ator) != pattern.charAt(ator))
            {
                missed++;
            }
        }while(++ator < pattern.length() && missed <= mismatches);

        if(missed <= mismatches)
        {
            System.out.println("Index: " + iter + " Pattern: " + text.substring(iter, iter + pattern.length()));
        }
    }

The output being indexes 0 bu, 2 bb, and 3 bl. The last two being mismatches with the tolerance of 1.

It looks fine. Some readability improvements:

I'd rename some variables first:

mismatches should be tolerance, mismatchTolerance or allowedMismatches. For me mismatches sounds like a counter but it never changes which is confusing somehow.
iter and ator do not make the code easier to read. textIndex (maybe searchIndex) and patternIndex would be better since they are more meaningful and explains the purpose.

Then extract out some named variables like

final int textIndexMax = text.length() - pattern.length() + 1;
final char textChar = text.charAt(textIndex + patternIndex);
final char patternChar = pattern.charAt(patternIndex);
final String match = text.substring(textIndex, textIndex + pattern.length());

Reference: Chapter 6. Composing Methods, Introduce Explaining Variable in Refactoring: Improving the Design of Existing Code by Martin Fowler

  Put the result of the expression, or parts of the expression,
  in a temporary variable with a name that explains the purpose.

The condition of the do-while loop could be rewritten as a usual (and a not so complex, therefore an easier to understand and less error-prone) for (i = 0; i < x; i++) loop and a break statement.

Here is my version:

final int mismatchTolerance = 1;
final String text = "bubbles";
final String pattern = "bu";

final int textIndexMax = text.length() - pattern.length() + 1;
for (int textIndex = 0; textIndex < textIndexMax; textIndex++) {
    int missed = 0;

    for (int patternIndex = 0; patternIndex < pattern.length(); patternIndex++) {
        final char textChar = text.charAt(textIndex + patternIndex);
        final char patternChar = pattern.charAt(patternIndex);
        if (textChar != patternChar) {
            missed++;
        }
        if (missed > mismatchTolerance) {
            break;
        }
    }

    if (missed <= mismatchTolerance) {
        final String match = text.substring(textIndex, textIndex + pattern.length());
        System.out.println("Index: " + textIndex + " Match: " + match);
    }
}


Fast insert, fast removal and fast access object pool C++ container I just want to gather some feedback about this container, in my view it solves the problem of common data containers which don't have fast access and fast deletion. I want to admit that I have not benchmark this, as I don't understand how to do it and to compare with other containers. I tested it and it works, although it might not be the cleanest or safest code.

map<> has O(log(n)) for access and deletion

vector<> has O(n) for deletion

As you can see, with usual STL containers, you can't have O(1) performance and low memory footprint everywhere.

I heard about object pool some time ago, where instead of deleting objects, you reuse them, but the problem is that you have to resize the vector<>.

Then I've read a suggestion on IRC about swapping the tail of the vector with the item I want to delete. The problem is that the index of the last item is changing.

So I wrote this container with an index mapper.

The index is just two vector<int>, a forward and a backward map, with a queue<int> for available indexes.

The data is a vector<T>.

Each time an item is remove or added, the index is adjusted so that you can still access the item by the same index.

template <typename T>
struct managed_pool {
    queue<size_t> avail;
    vector<T> data;
    vector<int> index, back_index; // forward and backward index
    size_t add(T&&val) {

        // always add to tail since we "swap-pop":
        data.push_back(val);
        size_t data_index = data.size() - 1;
        size_t index_index;

        // if there's no available index
        if (avail.empty()) {
            // add a new index
            index.push_back(data_index);
            index_index = index.size()-1;
        }
        else {
            // reuse one
            index_index = avail.front();
            avail.pop();
        }
        back_index.push_back(index_index);
        index[index_index] = data_index;

        msgm(val, index_index);
        return index_index;
    }
    void rem(size_t translate_index_delete) {
        auto real_index_backup = data.size() - 1; // you want to keep this
        auto real_index_delete = index[translate_index_delete]; // you want to delete this
        auto translate_index_backup = back_index[real_index_backup];

        index[translate_index_delete] = -1; // invalidate
        avail.push(translate_index_delete);

        data[real_index_delete] = data[real_index_backup]; // here we "swap-pop". We move the .back() of the vector to the index of the item we want to remove. This ensures contiguity of the vector, at a minimal performance cost
        data.pop_back(); // remove it

        index[translate_index_backup] = real_index_delete; // we adjust the mapping table to make sure the item is still indexed at the same index.
    }
    T&operator[](size_t & i) {
        if(index[1] != -1)
        return data[index[i]];
    }
};

I just want to have your opinion if this is good, and if there are better methods than this. Remember that a heap is not what I want. I want the fastest container and keep a low memory footprint, be simple enough, have fast insertion, fast removal, and fast access. I'm not sure queue<> is a good choice.

Define exactly what properties you want your container to have

You say:

  As you can see, with usual STL containers, you can't have O(1) performance and low memory footprint everywhere.

That's not so much a property of STL, but of container structures in general. You only get O(1) performance if you don't have to search and don't have to shuffle memory around, and if you can't shuffle memory around you can't have a low memory footprint. So you have to make some compromises.

You don't mention what you want to optimize for, or what kind of usage patterns you are expecting. It would be good if you could write down (for yourself) what exact properties you want your container to have, and then verify whether your implementation actually has those properties.

Your metadata uses storage as well

Your container has a vector<T> with the actual data, which is quite efficient. However, there is also metadata. In particular, there is vector<int> index, which you never shrink. So this means that if you have an access pattern where there is a short spike where a lot of data is stored in the container, then after the spike the data itself doesn't use a lot of space, but the vector of indices is now very large. Especially if T is small, then the overhead of the indices and availability queue might be very significant.

Note that you also never shrink index, even if it is possible to do so (whenever tail indices are unused).

Adding to a vector<> is amortized O(1)

Adding items to a vector<> might cause memory allocations and moves, so the time used for an addition is variable, and does not have an upper bound. Whenever the STL has to reallocate memory for a vector<>, it basically doubles the size, so with a constant rate of addition, it needs less and less reallocations. The result is that the amortized cost is O(1). However, be aware that this container might not be suitable for a real-time system.


Simplifying and optimize a concurrent access I have code that is accessed by multiple threads.  If by any chance it has a problem, all accessing threads will wait for recovery.  By default, the first thread that fails, will have to recover and all others will have to wait.  When the process is complete, everyone leaves the method.

[TestMethod]
public void RecoveryConcurrentTest()
{
    Task task = Task.Factory.StartNew(() =>
        {
            ManualResetEvent r1 = new ManualResetEvent(false);
            var t1 = new Thread((obj) =>
            {
                AutoRecover();
                r1.Set();
            });

            ManualResetEvent r2 = new ManualResetEvent(false);
            var t2 = new Thread((obj) =>
            {
                AutoRecover();
                r2.Set();
            });

            ManualResetEvent r3 = new ManualResetEvent(false);
            var t3 = new Thread((obj) =>
            {
                AutoRecover();
                r3.Set();
            });

            ManualResetEvent r4 = new ManualResetEvent(false);
            var t4 = new Thread((obj) =>
            {
                AutoRecover();
                r4.Set();
            });
            t1.Start();
            t2.Start();
            t3.Start();
            t4.Start();

            WaitHandle.WaitAll(new WaitHandle[] { r1, r2, r3, r4 });
        });
    task.Wait();
}

private Object _syncRoot = new Object();
private int count;
private bool handled;
public void AutoRecover()
{
    Trace.WriteLine("thread: " + Thread.CurrentThread.ManagedThreadId);
    Interlocked.Increment(ref count);
    Trace.WriteLine("count + 1 = " + count); // not work properly

    lock (_syncRoot)
    {
        Trace.WriteLine("thread: " + Thread.CurrentThread.ManagedThreadId);
        if (handled)
        {
            Trace.WriteLine("handled is true");
        }
        else
        {
            Trace.WriteLine("handled is false");
            Trace.WriteLine("PROCESSING....");
            Thread.Sleep(10000);
            Trace.WriteLine("PROCESSED....");
            handled = true;
            Trace.WriteLine("handled set true");
        }

        Interlocked.Decrement(ref count);
        Trace.WriteLine("count - 1 = " + count);
        if (count == 0)
        {
            handled = false;
            Trace.WriteLine("handled set false");
        }
    }
}

It works well for my tests, but I believe that there are necessary adjustments and that it can be implemented differently.

There is no sense in creating a task and waiting on it straight away. The following code is almost identical to your test method (almost - because tasks are created on a thread pool here):

[TestMethod]
public void RecoveryConcurrentTest()
{
    var tasks = Enumerable.Range(0, 10).Select(i => Task.Factory.StartNew(AutoRecover)).ToArray();

    Task.WaitAll(tasks);
}

Note that it doesn't actually test the fact that AutoRecover allows only one thread at a time, it just ensures that nothing fails when multiple threads call this method. There is no way in current design to detect it.

What I would suggest is to expose the fact of long process inside of AutoRecover by returning a Task that any thread can wait on:

[TestMethod]
public void RecoveryConcurrentTest()
{
    var tasks = Enumerable.Range(0, 10).Select(i => AutoRecoverAsync()).ToArray();

    Task.WaitAll(tasks);
}

private volatile Task _recoveryTask;
private readonly object _syncRoot = new object();

private async Task DoActualRecover()
{
    Trace.WriteLine("PROCESSING....");
    await Task.Delay(10000);
    Trace.WriteLine("PROCESSED....");
    _recoveryTask = null;
}

public Task AutoRecoverAsync()
{
    var recoveryTask = _recoveryTask;
    if (recoveryTask != null)
        return recoveryTask;

    lock (_syncRoot)
    {
        recoveryTask = _recoveryTask;
        if (recoveryTask != null)
            return recoveryTask;

        return _recoveryTask = DoActualRecover();
    }
}


Downloading list of images This code downloads a list of images, each in a different thread.

I have some questions about this code:

I don't know how to get information about correcting downloading (sometimes thread just stopped and doesn't do anything: no error, nothing and only part of image has been downloaded). I use the next mechanism for fixing: I wait some time and if thread is alive, I write this image on need_reload file. Is there a better solution?
Is it good or bad this using of threads for this task?
Are parts of my code not "pythonic"?

import os
from threading import Thread
import urllib
import requests

def write_to_failed_image_urls_file(file_name, image_url, failed_image_urls_file):
    """
    Check image in file and write it if need
    :param file_name: image file name
    :param image_url: image URL
    :param failed_image_urls_file: name of file with fails
    :return: None
    """
    with open(failed_image_urls_file, 'a+') as need_reload:
        need_reload.seek(0)
        lines = need_reload.readlines()
        founded = False
        for line in lines:
            if line.startswith(image_url):
                print('File is here')
                founded = True
                break
        if not founded:
            need_reload.write(image_url + "," + file_name + '\n')

def write_response_to_file(response, file_name):
    with open(file_name, 'wb') as f:
        for chunk in response.iter_content(chunk_size=2048):
            f.write(chunk)

def load_image_chunk(image_url, file_name, dir_):
    """
    Loading image by URL
    :param image_url: URL of image
    :param file_name: destination file name
    :param dir_: destination directory
    :return: None
    """
    r = requests.get(image_url, stream=True)
    if r.status_code == requests.codes.ok:
        try:
            write_response_to_file(r, file_name)
        except OSError as err_:
            print(err_.__str__(), 'try redownload...')
            file_name = os.path.join(dir_, file_name.split('=')[-1] + '.jpg')
            write_response_to_file(r, file_name)
    else:
        print(r)

def load_images(image_url_list, dir_, failed_image_urls_file, delay=5):
    """
    loading list of images
    :param image_url_list: list of image urls
    :param dir_: destination dir
    :param failed_image_urls_file: name of file with unsuccessful urls
    :param delay: delay for thread
    :return:None
    """
    abs_failed_image_urls_file = os.path.join(dir_, failed_image_urls_file)
    if not os.path.exists(abs_failed_image_urls_file):
        with open(abs_failed_image_urls_file, 'w') as _:
            pass
    for image in image_url_list:
        f = os.path.join(dir_, image.split('/')[-1])
        t = Thread(target=load_image_chunk, args=(image, f, dir_))
        t.start()
        t.join(delay)
        if t.isAlive():
            print('Bad, bad thread!')
            if abs_failed_image_urls_file is not None:
                write_to_failed_image_urls_file(f, image, abs_failed_image_urls_file)

I think this is pretty good code!  I have only a few minor suggestions:

Rename image and images to something that makes clear they are URLs, not files, not numpy ndarrays, and not any other format.  Maybe image_url and image_url_list?  The first time I read the code I didn't know what they were.  Your docstrings are good, but IMO, its even more Pythonic to have your code speak for itself (with docstrings present as backup obviously).
It took me several times of reading the code to figure out exactly what need_reload_file is.  Could you name it to be more descriptive?  Maybe failed_image_urls_file or something?
Web programmers might know what various request status codes mean, but in case a web noob like me reads your code, a comment explaining the meaning of r.status_code == 200 would be helpful.
I don't understand the file_name.split('=')[-1] bit.  What constraint are you assuming about the structure of file names?


Validate that a given string (name) meets certain requirements String Name = "yyyy MM DD abjwg kelk.exe"

public static bool IsNameRight(string Name)
{
    string[] temp = Name.Split(' ');
    string[] de= Name.Split('.');

    if (de[1] == "pdf" && temp .Length == 5)
    {
        if (temp [0].Length == 4
            && temp [1].Length == 2
            && temp [3].Length == 2
            && temp [4] == "abjwg"
            && temp [5] == "kelk.exe")
            return true;
        else
            return false;
    }
    return false;
}

I am using .NET 3.5 and C#

EDIT

You could use Regular expressions but I have never used it or know how to use it, and don't know if it is a good practice.

As you wanted to use RegEx I have a recommendation for you:

public static bool IsValidName(string Name)
{
    if(String.IsNullOrEmpty(Name))
        return new ArgumentNullException();

    string pattern = @"^[0-9]{4} [0-9]{2} [0-9]{2} abjwg kelk\\.exe$";
    return Regex.IsMatch(Name, pattern);
}


Scroll Magic Multiple Scenes I am using Scroll Magic for the first time and I am in general very new to JavaScript/jQuery. I got it to work but it is very messy and repetitive code.

Here is the fiddle. This is the messy code (I just gave this as an example, in my project there is about 50 scenes after each other in this manner!):

var controller = new ScrollMagic.Controller();

var tween1 = TweenMax.to('#animation1', 0.7, {
    opacity: 0
  });
var scene1 = new ScrollMagic.Scene({
    offset: 200,
    reverse: true
})
.setTween(tween1)
.addTo(controller)
.addIndicators();

var tween2 = TweenMax.to('#animation2', 0.7, {
    opacity: 1
  });
var scene2 = new ScrollMagic.Scene({
    triggerHook: 'onEnter',
    offset: 500,
    reverse: true
})
.setTween(tween2)
.addTo(controller)
.addIndicators();

Is there any way I can make this code more DRY?

I'm posting this with an assumption that the JsFiddle is missing something since it throws a "setTween is not a function" error. But given your code...

What you really want is a factory object, which gets provided some config data. Perhaps something in this format:

{
    "tween": {
        "duration": 0.7,
        "options": {
            "opacity": 0
        }
    },
    "scene": {
        "options": {
            "triggerHook": "onEnter",
            "offset": 500,
            "reverse": true
        }
    }
}

Let's just call this "animation factory" AnimationFactory:

function AnimationFactory() {
    this.controller = new ScrollMagic.Controller();
    this.scenes = [];
}

AnimationFactory.prototype = {

    controller: null,

    scenes: null,

    constructor: AnimationFactory,

    createAnimation: function(element, args) {
        element = typeof element === "string"
            ? document.getElementById(element)
            : element;

        var tween = TweenMax.to(element, args.tween.duration, args.tween.options),
            scene = new ScrollMagic.Scene(args.scene.options);

        scene.setTween(tween)
        scene.addTo(this.controller)
        scene.addIndicators();

        this.scenes.push(scene);

        return this;
    }

};

Then to use it:

var animationFactory = new AnimationFactory();

window.onload = function() {
    animationFactory
        .createAnimation("animation1", {
            tween: {
                duration: 0.7,
                options: {
                    opacity: 1
                }
            },
            scene: {
                options: {
                    offset: 200,
                    reverse: true
                }
            }
        })
        .createAnimation( ... );
};

Not you've got a nice clean API to use. But you said there are 50 of them on the page, and I imagine that number could change. Additionally, you have a disconnect between the animations and the elements that require them. Why not marry the two in wedded bliss using HTML5 custom data attributes?

function AnimationFactory() {
    this.controller = new ScrollMagic.Controller();
    this.scenes = [];
}

AnimationFactory.prototype = {

    controller: null,

    scenes: null,

    constructor: AnimationFactory,

    createAnimation: function(element, args) {
        ...
    },

    createAnimations: function(element) {
        var tween = null,
            scene = null,
            elements = element.querySelectorAll("[data-animation]"),
            i = 0,
            args;

        for (i; i < elements.length; i++) {
            args = JSON.parse(elements[i].getAttribute("data-animation"));
            this.createAnimation(elements[i], args);
        }
    }

};

And to use this:

var animationFactory = new AnimationFactory();

window.onload = function() {
    animationFactory.createAnimations(document.body);
};

And a little HTML for good measure:

Animation 1

Animation 2

This gives you unlimited potential. The configs for creating the animations are embedded right in the markup. If different pages had different animations, no need to keep different JavaScript files around. Every page can be different.

Since the AnimationFactory encapsulates all of the animations, you can tweak the work flow behind the scenes if you run into performance problems, like staggering the creation of the Tweens so you don't have 50 opacity tweens run all at once on the page (which I image might draw enough electricity from the CPU to power a small Tesla).

If you AJAX in some HTML, then insert it into a DIV after page load, the AnimationFactory#createAnimations method can be called, and pass in the HTML element that just had its innerHTML replaced and you'll get animations after AJAX, not just on page load.

// xhr is an XMLHttpRequest object that just got some HTML from the server
var div = $("#someDiv").html(xhr.responseText);

animationFactory.createAnimations(div[0]);


Simple 'Evolution of Text' app I'm a complete beginner to Python, and wanted to get some feedback on my first app. I tried to make a clone of the 'evolution of text' app found on the Using Python website.

  Enter some text, and watch the program 'evolve' the answer. The program just prints a load of random text, but keeps the correct bits each time.

I wrote my program without looking at the solution on the website. My solution is a little different, so I wanted to ask some more experienced developers what they think of it, what might be bad style and what could be improved.

import random
import string

target = list(raw_input("Enter your target text: "))
current = [random.choice(string.ascii_letters + string.digits + string.punctuation + ' ') for n in range(len(target))]
index = 0
counter = 0

while current != target:
    if current[index] != target[index]:
        current[index] = random.choice(string.ascii_letters + string.digits + string.punctuation + ' ')
        counter += 1
        print ' '.join(current)
    if index == len(target) - 1:
        index = 0
    else:
        index += 1

print("Done! Match took {0} generations.".format(counter))

Some suggestions:

Your algorithm is not doing the same thing as the original.  The original randomizes the entire string each time, except the correct bits, while yours randomizes only one character each time.
You can iterate directly over lists.  So you don't need range(len(target)), you can just do for _ in target.
You should define string.ascii_letters + string.digits + string.punctuation + ' ' as a new variable on a separate line so you can re-use it.
You can use the modulus operator % to keep the index inside the correct bounds.


Merge inserted and deleted tables in SQL trigger I have a table with a Balance column of type decimal(18,0).

I want to get updates to its Balance field and reflect the changes in the parents of the updated row.

Currently I can easily subtract parent values by selecting from deleted table and add to parent values by selecting from inserted table in two different queries. Since the updates will trigger an other update and so the concept of recursive triggers (enabled in my database) comes to scene, I want to have the minimum number of updates.

Is the following the best way to merge inserted and deleted table and to get the real difference in Balance field?

Update MyTable
Set Balance += updated.Price
From (
    Select  IsNull(inserted.ParentID, deleted.ParentID) As ParentID,
            (IsNUll(inserted.Balance,0) - IsNull(deleted.Balance,0))  As Price
            From inserted
            Full join deleted
            on inserted.ID = deleted.iD
            WHERE IsNull(inserted.ParentID, deleted.ParentID) IS NOT NULL
) As updated
INNER JOIN MyTable
ON MyTable.ID = updated.ParentID
Where updated.ParentID IS NOT NULL

For example changing Balance from 1000 to 800 should yield to -200 change in parent Balance fields.

Also, my code fires the trigger one extra time, so that for an update in level 3 I get 4 updates (trigger-firing) instead of 3.

This is on SQL Server 2012.

For me it is not exactly clear what you are trying to achieve, but it looks like propagating a balance change up into some tree (some chart of accounts, maybe?).

First, using triggers like this is very convoluted and error prone. Max recursion level for nested triggers is 32 and it is very hard to trace what it is going on.

Maybe the following approach might be useful for you: create a temporary table based on your data to update (you may have multiple updates at once)

ID
ParentID
LevelNo -- should contain the level in the tree
OriginalBalance
NewBalance

Data can be populated using a recursive CTE, if update is touching only one node and you want to operate on that branch only.

For each level = LevelNo to 1
   Update parent with balance difference

This suggestion works only if your structure is a tree. If you provide us more details (table structure etc), we may come with a more concrete solution.


Optimizing an avatar toggle I have a link (avatar)#qam-account-toggle and div.qam-account-items, the div containing user account links. I am toggling div on avatar click as well as click anywhere but div.qam-account-items

Here is my code. How can I optimize this better?

$('#qam-account-toggle').click(function(e){
    e.stopPropagation();
    $(this).toggleClass('account-active');
    $('.qam-account-items').fadeToggle('fast');
});

$(document).click(function(){
    $('#qam-account-toggle.account-active').removeClass('account-active');
    $('.qam-account-items:visible').hide();
});

$('.qam-account-items').click(function(event){
   event.stopPropagation();
});

Not much to say here, it looks pretty good. I've moved to only using .on for attaching events as I think it encourages better practices. Remember it's better to save selectors as variables if you are going to reuse them.

Instead of catching the click on .qam-account-items you can just fire the event on everything but those classes;

$(document).on('click', '*:not(.qam-account-items)', function() {
    $('#qam-account-toggle.account-active').removeClass('account-active');
    $('.qam-account-items:visible').hide();
});


Number of right cyclic shifts to give the biggest number I am preparing to coding interview and I met this task today:

You are given 30-bit unsigned integer N. A right cyclic shift of N by K bits is the result of performing a right cyclic shift of N by one bit K times. Leading zeros may appear.

For example:

the right cyclic shift of 9736 by one bit is 4868,
the right cyclic shift of 9736 by two bits is 2434,
the right cyclic shift of 9736 by eleven bits is 809500676

The number 809500676 is the largest value that can be obtained by performing a right cyclic shift of 9736.

The aim is to find integer K such that right cyclic shift of N by K bits gives the largest possible value. In example above method should return 11. 0<=N<=1073741823. Worst-case time complexity is O(log(N)).

My try should be correct value but does not meet (in my opinion, but I am not sure) time complexity.

    public int solution(int N) {
        long m = N;
        long max = N;
        int res = 0;
        for(int i =1;i<30;i++) {
            m=(N>>>i) & 0x3fffffff | (N<<(30-i)& 0x3fffffff);
            if(m>max) {
                max=m;
                res=i;
            }
        }
        return res;
    }

Your solution seems fine.
I would suggest some coding style improvements though:

To avoid errors, avoid magic numbers, put them in constants, for example:

MAX_N = 0x3fffffff
MAX_SHIFT = 30

The variable m doesn't need to be initialized before the loop. In fact, it doesn't need to be declared before the loop. Move it inside.
res is not a great name to store the number of shifts to get the highest number. shiftCount would be better.
solution is not a great name for the method to get the shift count for the highest number. getShiftCountToHighestNum would be better.

With the above suggestions applied, the code becomes:

private static final int MAX_N = 0x3fffffff;
private static final int MAX_SHIFT = Integer.toBinaryString(MAX_N).length();

public int getShiftCountToHighestNum(int N) {
    long max = N;
    int shiftCount = 0;
    for (int i = 1; i < MAX_SHIFT; i++) {
        long m = (N >>> i) & MAX_N | (N << (MAX_SHIFT - i) & MAX_N);
        if (m > max) {
            max = m;
            shiftCount = i;
        }
    }
    return shiftCount;
}

As for the time complexity,
I believe this is indeed \$O(\log N)\$.
The number of calculation steps is proportional to the number of bits in \$N\$, which grows much slower than \$N\$ itself.


Rails validating API parameters I wanted to keep my API request logic separate form the controller logic. I therefore make use of a separate model (EmailChecker) that creates an instance of this class via EmailChecker.new(params).

Then I can check the new instance with valid?.

When it returns false I grep the errors and return it. This example only checks email but of course you can use any Rails validations on multiple parameters.

Problem is that I have to create a class for every API request you want to validate. Actually I don't mind because it makes the code easy to understand but it's not very DRY.

Any suggestions to improve this code?

class EmailChecker

  include ActiveModel::Validations
  include ActiveModel::Conversion
  include ActiveModel::Naming

  attr_accessor :email

  validates_format_of :email, :with => /\A[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}\z/i
  # validates :email, presence: true

  def initialize(attributes = {})
    attributes.each do |name, value|
      send("#{name}=",value)
    end
  end

  def persisted?
    false
  end

end #emailchecker

You've essentially created a "View Model" for your API requests, which there's nothing wrong with doing that. Sometimes you want to abstract away your internal Domain Model and not have it exposed as params in the request.

That being said, you can DRY things up a bit by creating a base class:

class ApiModel
  include ActiveModel::Validations
  include ActiveModel::Conversion
  include ActiveModel::Naming

  EMAIL_FORMAT = /\A[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}\z/i

  def initialize(attributes = {})
    attributes.each do |name, value|
      raise KeyError, "Attribute key '#{name}' is not supported" unless respond_to? "#{name}="
      send "#{name}=", value
    end
  end

  def persisted?
    false
  end
end

Now your EmailChecker class becomes a 4-liner:

class EmailChecker < ApiModel
  attr_accessor :email
  validates_format_of :email, :with => EMAIL_FORMAT
end

Next, I'd like to focus on the initialize method. In your version:

def initialize(attributes = {})
  attributes.each do |name, value|
    send("#{name}=",value)
  end
end

The send method call is left unguarded. You can specify keys in the Hash that do not correspond to setter methods in your object, leaving developers scratching their heads about why a method is not supported. Instead, this is an opportunity to fail early, and fail loud[er]. I would test to see if the attribute key is a valid setter, and then throw your own exception:

def initialize(attributes = {})
  attributes.each do |name, value|
    raise KeyError, "Attribute key '#{name}' is not supported" unless respond_to? "#{name}="
    send "#{name}=", value
  end
end

Raising a KeyError is more appropriate here, because the real problem is having an incorrect key in the attributes argument. The error message is informative to developers so they can fix the problem, yet does not give away any secrets.

For instance, if you send { foo: 'bar' } as the attributes, then you'll get the following error message:

  KeyError: Attribute key 'foo' is not supported

The persisted? method is kind of confusing to me. This object is not persisted, so why even have this method? Unless this is overriding functionality baked in from the ActiveModel mixins, I would just remove it.


Fetch StackOverflow questions and save them to a markdown table I'm trying to write a script that would (a) fetch StackOverflow questions and (b) update a markdown table with new entries.
With github actions I automated this script to run daily.
Idea borrowed from simonw
Code:
import re
import requests
from pathlib import Path
from datetime import datetime

URL = "https://api.stackexchange.com/2.2/questions"
DATE = datetime.utcnow().date()
ROOT = Path(__file__).parent.resolve()

def get_epochs(date):
    """ Get epoch dates for the start and end of the (current) day. """

    start = datetime(
        year=date.year, month=date.month, day=date.day,
        hour=0, minute=0, second=0
    )
    end = datetime(
        year=date.year, month=date.month, day=date.day,
        hour=23, minute=59, second=59
    )
    return int(start.timestamp()), int(end.timestamp())

def fetch_questions(start, end, tag, site="stackoverflow"):
    """ Fetch questions from stackoverflowAPI. """

    _params = {
        "fromdate": start,
        "todate": end,
        "order": "desc",
        "sort": "votes",
        "tagged": tag,
        "site": site,
    }
    return requests.get(URL, params=_params).json()

def build_table(*args, **kwargs):
    """ Build a markdown table from a list of entries. """

    columns = [
        "\n".join(
            "* [{title}]({url}) - {score} votes".format(
                # to prevent code from breaking if special characters are present
                title=re.sub(r'[^\w\s]', '', item["title"]),
                url=item["link"],
                score=item["score"]
            )
            for item in chunk["items"][:8]
        )
        for chunk in args
    ]
    return columns

def replace_chunk(content, marker, chunk, inline=False):
    """ Replace chunks of README.md """

    r = re.compile(
        r"<!\-\- {} starts \-\->.*<!\-\- {} ends \-\->".format(marker, marker),
        re.DOTALL,
    )
    if not inline:
        chunk = "\n{}\n".format(chunk)
    chunk = "{}".format(marker, chunk, marker)
    return r.sub(chunk, content)

if __name__ == "__main__":

    readme = ROOT / "README.md"
    start, end = get_epochs(DATE)

    pandas, beautifulsoup, code_review = build_table(
        fetch_questions(start, end, tag="pandas"),
        fetch_questions(start, end, tag="beautifulsoup"),
        fetch_questions(start, end, tag="python", site="codereview")
    )

    readme_contents = readme.open().read()
    rewritten = replace_chunk(readme_contents, "date", DATE.strftime("%Y-%m-%d"), inline=True)
    rewritten = replace_chunk(rewritten, "pandas", pandas)
    rewritten = replace_chunk(rewritten, "bs", beautifulsoup)
    rewritten = replace_chunk(rewritten, "code_review", code_review)

    with open(readme, "w") as output:
        output.write(rewritten)

Markdown file:
# Stackoverflow daily top questions:  date

### Pandas

pandas content

### BeautifulSoup

bs4 content

### Python code review submissions

code review content

Sample output:

Things look good and well organised at first glance. However, there are a few things you can make use of.

Use f-string over using str.format. The former is more performant.
You can group the columns and iterate over them, instead of having multiple lines for each section. This way, you can add/remove to this iterable without having to change much of the code itself.
When using raw strings, you do not need to escape the - character, unless they are being used inside a character set (for regex).
The build_table can be split to another separate function.
Use type hinting.
If you're not using kwargs, no need to declare them.

Rewritten snippets (you might need to fit them in your code accordingly):
sections = (
    {'tag': "pandas", "marker": "pandas"},
    {'tag': "beautifulsoup", "marker": "bs"},
    {'tag': "python", "site": "codereview", "marker": "code_review"},
)
for section in sections:
    questions_list = fetch_questions(start, end, **section)
    # Using `**section` might throw an error for the unknown kwarg: `marker`.
    # But it is trivial to handle that.
    .
    ...

def replace_chunk(content: str, marker: str, chunk: str, inline: bool = False):
    """ Replace chunks of README.md """
    r = re.compile(
        rf"<!-- {marker} starts -->.*<!-- {marker} ends -->",
        re.DOTALL,
    )
    if not inline:
        chunk = "\n{}\n".format(chunk)
    chunk = f"<!-- {marker} starts -->{chunk}<!-- {marker} ends -->"
    return r.sub(chunk, content)

def get_item_string(item):
    # to prevent code from breaking if special characters are present
    title = re.sub(r'[^\w\s]', '', item["title"])
    return f"* [{title}]({item['link']}) - {item['score']} votes"

def build_table(*args):
    """ Build a markdown table from a list of entries. """
    columns = [
        "\n".join(
            map(get_item_string, chunk["items"][:8])
        )
        for chunk in args
    ]
    return columns


Coursera: Calculating sum of numbers in the fibonacci sequence between two specific points in the series Question

This is an assignment for a Coursera course.

This is the algorithm I wrote.

import java.util.*;

public class FibonacciPartialSum {
    private static long getFibonacciPartialSumNaive(long from, long to) {
        if (to <= 1)
            return to;

        long prev = 0;
        long cur = 1;
        long sum;

        if(from == 1) {
            sum = 1;
        }
        else {
            sum = 0;
        }

        for (long i = 2; i <= to; i++) {
            long temp_prev = prev;
            prev = cur;
            cur = (cur + temp_prev) % 10;

            if (i >= from) {
                sum = (sum + cur) % 10;
            }

        }
        return sum;
    }

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        long from = scanner.nextLong();
        long to = scanner.nextLong();
        //long from = 1234;
        //long to = 12345;
        System.out.println(getFibonacciPartialSumNaive(from, to));
        scanner.close();
    }
}

When I submit it to auto grader in Coursera it states that I failed the time test.

Failed case #8/12: time limit exceeded Input: 1 10000000000

Your output:

stderr:

(Time used: 3.03/1.50, memory used: 26427392/536870912.)

Runtime improvement

If you take a look at the last digit of the first few fibonacci numbers you will see that the digits are repeated after 60 numbers. As the digits in this range have a sum of 0 you can calculate the sum for [from%60,to%60] instead.

Bug

The task states that the given integer are non-negative, 0 is a valid value. Currently you are returning i.e. 1 for the range [0,2] as 0 is treated as 2, the initialization of sum has to be changed to:

if (from <= 1) {
    sum = 1;
} else {
    sum = 0;
}

(I would initialize sum with a ternary operator long sum = from <= 1 ? 1 : 0 instead.)

Types

As the values of sum, prev and cur are always in the range [0,10), you can use int instead of long for these variables, int arithmetic is likely to be faster than long arithmetic.

Alternative implementation

The remaining range is quite small -> you could cache the results for every number in the range [0,60) to avoid calculating the sum on each invocation.

private static final byte[] DIGIT_SUM = {
        0, 1, 2, 4, 7, 2, 0, 3, 4, 8,
        3, 2, 6, 9, 6, 6, 3, 0, 4, 5,
        0, 6, 7, 4, 2, 7, 0, 8, 9, 8,
        8, 7, 6, 4, 1, 6, 8, 5, 4, 0,
        5, 6, 2, 9, 2, 2, 5, 8, 4, 3,
        8, 2, 1, 4, 6, 1, 8, 0, 9, 0};

private static int fibonacciPartialSum(long m, long n) {
    assert m <= n & (m | n) >= 0;
    int v = fibSumLastDigit(n);
    return m == 0 ? v : (v -= fibSumLastDigit(m - 1)) + (10 & v >> -1);
}

private static int fibSumLastDigit(long n) {
    return DIGIT_SUM[(int) (n % 60)];
}


Matrix to calculate effects using delta method I have written some code to calculate effects using the Delta method.

I have a dataframe dpcp with variables x1, x2, x3, x4 and a matrix of 1000 draws from a multivariate normal, m4[1000,4].

This code works to calculate the effects, but it runs very slowly - it currently takes upwards of 5 hours for only 2000 observations.  How can I improve the runtime?

n=nrow(dpcp)
for (i in 1:n)

 {

     for (j in 1:1000)
      {
     marg_effects[i,j]=(m4[j,1]*dpcp[i,]$x1)+(m4[j,2]*dpcp[i,]$x2)+(m4[j,3]*dpcp[i,]$x3)+(m4[j,4]*dpcp[i,]$x4)
 }
   }

Aren't you looking for a simple matrix multiplication? Try:

x <- as.matrix(dpcp[c("x1", "x2", "x3", "x4")])

Then

marg_effects <- x %*% t(m4)

or equivalently (maybe a little faster):

marg_effects <- tcrossprod(x, m4)


Basic modular calculator I am new to modular JavaScript code, and after reading an article on the Internet, I wrote a very basic calculator. This works fine, but due to some unknown reason, I feel that this code is not well written. I will appreciate it if someone could improve my code below so that it will be helpful with learning modular JavaScript.

$(function () {
    $('.button').on('click', function() {
        var operator = $(this).attr('name'),
        calc = new Calculator('output', 'valOne', 'valTwo', operator);

        calc.init();
    });
});

calculator.js

var Calculator = function(eq, valone, valtwo, operator) {
    var eqCtl = document.getElementById(eq),
    valone    = document.getElementById(valone),
    valtwo    = document.getElementById(valtwo),
    op        = operator,

    init = function() {
        op   = operator;
        val1 = parseInt($(valone).val());
        val2 = parseInt($(valtwo).val());

        calculation();
    },

    setVal = function(val) {
        eqCtl.innerHTML = val;
    },

    calculation = function() {
        if(op == 'add') {
            addition(val1, val2);
        }
        else if(op == 'sub') {
            subtract(val1, val2);
        }
        else if(op == 'mult') {
            multiply(val1, val2);
        }
        else {
            division(val1, val2);
        }
    },

    addition = function(x,y) {
        return setVal(x + y);
    },

    subtract = function(x,y) {
        return setVal(x - y);
    },

    multiply = function(x,y) {
        return setVal(x * y);
    },

    division = function(x,y) {
        if( y == 0 ) {
                return setVal('cannot divide by 0');
        } else {
                return setVal(x / y);
        }
    };

    return {
        init: init
    };
};

The concept of creating a new Calculator() with a specified operator and then calling calc.init() to actually perform the operation seems a bit strange. I'd probably have any initialisation built into the constructor, and then have a calc.calculate() method that takes the operation as a parameter.

I don't have time now for a detailed analysis of your code, but one thing I would probably do is make the different operation functions methods of an object:

var operations = {
    "addition" : function(x,y) { return setVal(x + y); },
    "subtract" : function(x,y) { return setVal(x - y); },
    "multiply" : function(x,y) { return setVal(x * y); },
    // etc
}

Because then you can eliminate the if/else structure that decides what function to call and just do this:

calculation = function() {
    if (op in operations)
        operations[op](val1, val2);
    else
        // invalid op requested, so show message, throw exception, whatever
}

If you add more operations in the future, say a toThePowerOf() operation, you'd add it to the operations object but wouldn't need to change the calculation() function.


Python Tabletop Initiative Tracker I wrote my first Python program today after working through a few books. I decided to build a group initiative tracker for our D&D group and was hoping to get some feedback. Critiques on code efficiency, formatting, suggestions, etc would be appreciated. My plan is to eventually take this from a command-line application to a django or flask web app, or maybe an iOS app.

print('Welcome to Initiative Tracker!')
while True:
    try:
        loop_count=int(input("\nHow many PC's and NPC's total? "))
        loop_start = 0
        initiative_tracking = {}
        while loop_start < loop_count:
            player_name = input('What is the Players name? ')
            player_initiative = int(input('What is ' + player_name + ' initiative? '))
            if player_initiative in initiative_tracking:
                initiative_tracking[player_initiative].append(player_name)
            else:
                initiative_tracking[player_initiative] = [player_name]
            loop_start += 1
            continue
        print('\nYour initiative order is: ')
        for key in sorted (initiative_tracking.keys(), reverse=True):
            print(str(key) + ': ' + ', '.join(initiative_tracking[key]))
        break
    except ValueError:
        print("Sorry I didn't catch that. How many PC and NPC total? ")

I see that you have some error handling for int(input(...)).  I would change it, however.  Let's say that the user types a valid number of PC's, but does not type a valid player initiative.  That would trigger a ValueError, and your except block would ask again how many PC's.  Why should it restart the whole process for one little mistake?  You should have different try blocks for each of those. Also, a try block should include only the code that you expect might throw an error.  If some other code throws an error, you want to know about it.

For the first use of int(input(...)), I would just take it out completely.  Why should the user need to specify how many PC's?  Why not just keep going until he runs out and then hit Enter on a blank line?  That can be done quite simply with iter().  Usually, iter() is given an iterable, but if it is given two arguments,  the first argument is a callable (usually a function) that returns a value.  (In our case, we use lambda: input(...)).  The second argument is the stopping point.  iter() will keep calling that function and yielding the result until it returns the second argument. Therefore, we give it '', and we get all of the user's inputs until he types nothing in a line.

The second use of int(input(...)) is when you are retrieving the player initiative.  It should have its own try-except block to make sure the input is an integer.  I would then put that block inside of its own while loop so that the program keeps asking until it gets a valid answer.

Instead of a chain of string concatenations, use string formatting:

player_initiative = input('What is {} initiative? '.format(player_name))

and

print('{}: {}'.format(key, ', '.join(initiative_tracking[key])))

A loop is called that because it loops.  You don't need to tell it to keep looping.  It will keep looping until it is told to stop.  Sometimes it is stops because a condition is not met, but sometimes it stops because it encounters a break.  You should use continue only if you want to skip the code beneath and jump straight to the beginning of the loop.  It really doesn't make sense to put continue at the end of the loop.

The argument that is given to sorted() has only one requirement: that it is iterable.  To test if a type of object is iterable, go into a Python shell and create an instance of it.  Then, do for item in my_object: print(item).  If there is an error, your object is not iterable.  Otherwise, you will see what happens when iterates.  For a dictionary, you get all of the keys.  Therefore, sorted(initiative_tracking.keys(), reverse=True) can be simplified to sorted(initiative_tracking, reverse=True).

Full code:

print('Welcome to Initiative Tracker!')

initiative_tracking = {}
for player_name in iter(lambda: input("What is the player's name? "), ''):
    player_initiative = input('What is {} initiative? '.format(player_name))
    while True:
        try:
            player_initiative = int(player_initiative)
            break
        except ValueError:
            player_initiative = input('Please enter an integer: ')

    if player_initiative in initiative_tracking:
        initiative_tracking[player_initiative].append(player_name)
    else:
        initiative_tracking[player_initiative] = [player_name]

print('\nYour initiative order is: ')
for key in sorted(initiative_tracking, reverse=True):
    print(str(key) + ': ' + ', '.join(initiative_tracking[key]))


Min function accepting varying number of arguments in C++17 Come across this problem once again in the book The Modern C++ Challenge.

18. Minimum function with any number of arguments
Write a function template that can take any number of arguments and returns the minimum value of them all, using operator < for comparison. Write a variant of this function template that can be parameterized with a binary comparison function to use instead of operator <.

I wonder how simple and elegant the implementation could be using c++17. The following is the parameterized version. Ideas?
#include <algorithm>

template <typename Less, typename T, typename... Ts>
constexpr const T& min(Less less, const T& a, const T& b, const Ts&... rems) {
  if constexpr (sizeof...(rems)) {
    return min(less, std::min(a, b, less), rems...);
  }
  else {
    return std::min(a, b, less);
  }
}

template <typename Less, typename T, typename... Ts>
constexpr const T& min(Less less, const T& a, const T& b, const Ts&... rems) {

This function requires a minimum of 2 elements. A minimum element will exist if the user provides a single argument variadic list. Consider handling that.

auto& min1 = min(std::less<>{}, 4, 5);  // okay!
auto& min2 = min(std::less<>{}, 4);     // candidate function not viable, reqs 3 args, 2 provided

        return min(less, std::min(a, b, less), rems...);

Your approach here uses recursion and implementations are permitted to put a recursion depth limit on constexpr calculations. Consider an iterative solution  that expands the pack while calculating the minimum.

template <typename Comparator, typename First, typename... Rest>
constexpr decltype(auto) variadic_min(Comparator cmp, First const& first, Rest const&... rest) {
    const First* result = std::addressof(first);

    // cast each evaluated expr to void in case of overloaded comma operator shenanigans
    ((void)(result = std::addressof(std::min(*result, rest, cmp))), ...);

    return *result;
}

An explanation with what is going on with the fold expression:

((void)(result = std::addressof(std::min(*result, rest, cmp))), ...);
 ^     ^         ^                                            ^
 |     |         |                        expand using comma op
 |     |         safer than built-in, now constexpr in 17
 |     evaluate the expression for each pack variable
 cast to void the entire expression to avoid overloaded comma op.

Just thinking beyond the standard library and reinventing the wheel. This min works fine for homogenous packs. What about heterogenous packs?

auto& min1 = min(std::less<>{}, 4, 5, -1);  // min1 = -1
auto& min2 = min(std::less<>{}, 4, 5, -1.); // candidate template ignored...

One of the benefits of the conditional operator (?:) is that if both resulting expressions return lvalues of the same type, then the result will have the same type. If that type is T&, we could assign to that variable. Could we mimic that behavior with min and friends?

auto a = 4;
auto b = 5;
((b < a) ? b : a)  = 42;        // a = 42, b = 5
min(std::less<>{}, a, b) = 42;  // cannot assign to return value, returns const-qualified type
std::min(a, b) = 42;            // same with standard library.
min(std::less<>{}, 5, 6) = 42   // cannot assign, makes sense!


Optimizing the reading of web.config Here's the issue.  When I build an asp.net application I create a singleton to read the values from the web.config file so that the values are only read once and gives a slight speed increase to the app.  I basically do the following :

/// <summary>
/// Static class that handles the web.config reading.
/// </summary>
public class SiteGlobal
{
    /// <summary>
    /// Property that pulls the connectionstring value.
    /// </summary>
    public static string ConnectionString { get; protected set;}

    /// <summary>
    /// Property that pulls the directory configuration value.
    /// </summary>
    public static string Directory { get; protected set; }

    /// <summary>
    /// Static contructor for the SiteGlobal class.
    /// </summary>
    static SiteGlobal() {
        ConnectionString = WebConfigurationManager.ConnectionStrings["ApplicationServices"].ConnectionString;
        Directory = WebConfigurationManager.AppSettings["Directory"];

    }
}

Which I don't consider a bad optimization.  (Suggestions for improvement are appreciated through.)

Now though I'm trying to read from a ConfigurationSection that can contain multiple area's for example :

<ApplicationArea>
    <test1>
        <add key="key value" value="value data"/>
    </test1>
    <test2>
        <add key="key value1" value="value data2"/>
    </test2>
</ApplicationArea>

Where I can have multiple area's underneath ApplicationArea, such as test1 and test 2 above.  The best I've been able to come up with for an optimization of this though is this :

public static string ApplicationArea(string skey, string skey2)
    {
        NameValueCollection nvc = WebConfigurationManager.GetSection("ApplicationArea/" + skey) as NameValueCollection;
        return nvc[skey2];
    }

Which is not saving the data and rereads the web.config file every time it's accessed.  Is there an easy way to optimize the reading of the sections?

The web.config is loaded into memory once per-request anyway - it isn't loaded and parsed every time you access a section via the WebConfigurationManager. What speed increases have you actually noticed?

In fact, it might not even occur for each request, according to MSDN: 'These settings are calculated once and then cached across subsequent requests. ASP.NET automatically watches for file changes and re-computes the cache when any of the configuration files change within that file's hierarchy. When the server receives a request for a particular URL, ASP.NET uses the hierarchy of configuration settings in the cache to find the requested resource.' So, you're just doubling up on memory consumption; and I'm not convinced it's faster.


Cuts up Python strings Please tell me what you think of this its my first try and I have only been learning Python for 3 days now, works with both 'index' and 'find':

data = ["XBOX 360 | 150 | New",
        "PlayStation 3 | 35 | Broken",
        "Windows 8 | 275 | New"]

Id = 0
one = data[Id].index("|")
two = data[Id].index("|", one+1)

product = data[Id][:one]
price = data[Id][one+2:two]
condition = data[Id][two+2:]

print(product)
print(price)
print(condition)

#Just add more like these:
#three = data[Id].index("|", two+1)
#four = data[Id].index("|", three+1)
#etc... If you want to add more 'Pipes'
#Add one to each 'pipe' no more then that or you'll get an error

#You can add as many new 'data' items as you want
#just change the 'Id' to view them

First, there is no need to use index() method for finding the position of next and next delimiter (and then use it for slicing) - Python is able to split the string at a higher level - pinpoint the delimiter and then use the split() method:

   DELIMITER = " | "                        # we will use it for splitting the string

   "XBOX 360 | 150 | New".split(DELIMITER)  # the result will be ["XBOX 360", "150", "New"]

As you may see from the comment, the result is a list of individual parts.

Second - so we now have the list of the appropriate parts of the string.
Let give it the name parts - so parts will be the list:

   ["XBOX 360", "150", "New"]

(You may obtain it by the command parts = data[0].split(DELIMITER) in your code.)

Now instead of using indices for getting individual parts, i. e. "XBOX 360", "150", and "New":

   product   = parts[0]
   price     = parts[1]
   condition = parts[2]

you may just unpack the parts list:

   product, price, condition = parts

Third, fourh, fifth, ... - I'm not going to overload you :-)


Append nones to list if is too short Is it possible to improve this bit?

def append_nones(length, list_):
    """
    Appends Nones to list to get length of list equal to `length`.
    If list is too long raise AttributeError
    """
    if len(list_) < length:
        nones = length - len(list_)
        return list_ + [None] * nones
    elif len(list_) > length:
        raise AttributeError('Length error list is too long.')
    else:
        return list_

1. Comments on your code

The documentation does not match the implementation. The docstring says "append Nones to list" but it does not append: if the list does not have the required length, it returns a newly allocated list.
It seems wrong to return the original list if it was the right length, but return a newly allocated list if it was too short. For consistency, the function should either always return a new list, or always (update and) return the old list.
It's a loss of generality to support only the addition of None elements. Why not let the caller pass in their preferred placeholder element.
If you run help(AttributeError) you'll see that its description is "Attribute not found." This does not seem appropriate here. ValueError would be a better choice, or better still, create your own.
There's no need to name the variable list_: this avoids shadowing the built-in function list, but you don't use that function, so it doesn't matter if you shadow it.
You call len(list_) three times. Simpler to call it once and store the value.
The case len(list_) == length could be combined with the case len(list_) < length and so simplify the code.
The error message "Length error list is too long" makes no sense in English.
The error message would be more useful if it contained the actual numbers.
This kind of function is ideal for doctests.

2. Revised code

I've chosen to make the function always update its argument, and fixed all the problems noted above.

class TooLongError(ValueError):
    pass

def pad(seq, target_length, padding=None):
    """Extend the sequence seq with padding (default: None) so as to make
    its length up to target_length. Return seq. If seq is already
    longer than target_length, raise TooLongError.

    >>> pad([], 5, 1)
    [1, 1, 1, 1, 1]
    >>> pad([1, 2, 3], 7)
    [1, 2, 3, None, None, None, None]
    >>> pad([1, 2, 3], 2)
    ... # doctest: +IGNORE_EXCEPTION_DETAIL
    Traceback (most recent call last):
      ...
    TooLongError: sequence too long (3) for target length 2

    """
    length = len(seq)
    if length > target_length:
        raise TooLongError("sequence too long ({}) for target length {}"
                           .format(length, target_length))
    seq.extend([padding] * (target_length - length))
    return seq


DSSudokuSolver - A JavaScript Sudoku solving algorithm I wrote DSSudokuSolver - a sudoku solving algorithm a while back. Is there any possibility that this algorithm can be improved?

Original Algorithm:

CleanElements = function(comp_ary, Qsudoku){
    for(i=0; i<9; i++){
        for(j=0; j<9; j++){
            /*if(Qsudoku[i][j] != ""){
              comp_ary[i][j]=[];
              }*/
            for(k=0; k<9; k++){
                i_index = comp_ary[i][k].indexOf(Qsudoku[i][j]);
                if(i_index != -1){
                    comp_ary[i][k].splice(i_index, 1);
                }
                j_index = comp_ary[k][j].indexOf(Qsudoku[i][j]);
                if(j_index != -1){
                    comp_ary[k][j].splice(j_index, 1);
                }
            }
            if(i < 3){
                i_min = 0;
                i_max = 2;
            }
            else if(i < 6){
                i_min = 3;
                i_max = 5;
            }
            else{
                i_min = 6;
                i_max = 8;
            }

            if(j < 3){
                j_min = 0;
                j_max = 2;
            }
            else if(j < 6){
                j_min = 3;
                j_max = 5;
            }
            else{
                j_min = 6;
                j_max = 8;
            }

            for(i_box=i_min; i_box<=i_max; i_box++){
                for(j_box=j_min; j_box<=j_max; j_box++){
                    index = comp_ary[i_box][j_box].indexOf(Qsudoku[i][j]);
                    if(index != -1){
                        comp_ary[i_box][j_box].splice(index, 1);
                    }
                }
            }
        }
    }
    return comp_ary;
}

FindElements = function(comp_ary, Qsudoku){
    for(i=0; i<9; i++){
        for(j=0; j<9; j++){
            if(comp_ary[i][j].length == 1){
                if (Qsudoku[i][j] == ""){
                    Qsudoku[i][j] = comp_ary[i][j][0];
                    comp_ary[i][j] = [];
                }
            }
        }
    }
    return Qsudoku;
}

IsThereNullElement = function(Qsudoku){
    for(i=0; i<9; i++){
        for(j=0; j<9; j++){
            if(Qsudoku[i][j] == ""){
                return false;
            }
        }
    }
    return true;
}

InitEmptyArray = function(){
    empty_ary = Array();
    for(i=0; i<9; i++){
        empty_ary[i] = Array();
        for(j=0; j<9; j++){
            empty_ary[i][j] = Array();
            for(k=0; k<9; k++){
                empty_ary[i][j][k] = (k+1).toString();
            }
        }
    }
    return empty_ary;
}

DSSolve = function(Qsudoku){
    comp_ary = InitEmptyArray(); //Complementary Array
    window.comp_ary_old = comp_ary;
    IterationMax = 5000;

    while(true){
        IterationMax -= 1;
        comp_ary = CleanElements(comp_ary, Qsudoku);
        console.log(comp_ary);

        if(window.comp_ary_old == comp_ary){
            //implement this.
        }
        else{
            window.comp_ary_old = comp_ary;
        }

        Qsudoku = FindElements(comp_ary, Qsudoku);
        //console.log(Qsudoku);

        if(IsThereNullElement(Qsudoku)){
            return Qsudoku;
        }

        if(IterationMax == 0){
            return null;
        }
    }
}

It's not a huge improvement, just taking a stab at a few slight tweaks:

var sudoku = {
    CleanElements:function(comp_ary, Qsudoku){
        var i_factor,
            j_factor,
            i_min,
            i_max,
            i_index,
            j_index,
            index;

        for(var i=9; i--;){
            i_factor = (3*Math.floor(i/3));
            i_min = 6 - i_factor;
            i_max = 8 - i_factor;

            for(var j=9; j--;){
                j_factor = (3*Math.floor(j/3));
                j_min = 6 - j_factor;
                j_max = 8 - j_factor;

                for(var k=9; k--;){
                    i_index = comp_ary[i][k].indexOf(Qsudoku[i][j]);
                    j_index = comp_ary[k][j].indexOf(Qsudoku[i][j]);

                    if(i_index !== -1){
                        comp_ary[i][k].splice(i_index,1);
                    }

                    if(j_index !== -1){
                        comp_ary[k][j].splice(j_index,1);
                    }
                }

                for(var i_box=i_max; i_box>=i_min; i_box--){
                    for(var j_box=j_max; j_box>=j_min; j_box--){
                        index = comp_ary[i_box][j_box].indexOf(Qsudoku[i][j]);
                        if(index !== -1){
                            comp_ary[i_box][j_box].splice(index, 1);
                        }
                    }
                }
            }
        }
        return comp_ary;
    },
    FindElements:function(comp_ary, Qsudoku){
        for(var i=9; i--;){
            for(var j=9; j--;){
                if(comp_ary[i][j].length === 1){
                    // in case you were specifically checking that it was an empty string and not a null / undefined / etc, change to Qsudoku[i][j] === ''
                    if (Qsudoku[i][j].length === 0){
                        Qsudoku[i][j] = comp_ary[i][j][0];
                        comp_ary[i][j] = [];
                    }
                }
            }
        }
        return Qsudoku;
    },
    IsThereNullElement:function(Qsudoku){
        for(var i=9; i--;){
            for(var j=9; j--;){
                // same here, change to === '' if specifically needed
                if(Qsudoku[i][j].length === 0){
                    return false;
                }
            }
        }
        return true;
    },
    InitEmptyArray:function(){
        var empty_ary = Array();

        for(var i=9; i--;){
            empty_ary[i] = Array();

            for(var j=9; j--;){
                empty_ary[i][j] = Array();

                for(var k=9; k--;){
                    empty_ary[i][j][k] = (k+1)+'';
                }
            }
        }
        return empty_ary;
    },
    DSSolve:function(Qsudoku){
        var self = this,
            comp_ary = self.InitEmptyArray(),
            Qsudoku;

        this.comp_ary_old = comp_ary;

        for(var i=5000; i--;){
            comp_ary = self.CleanElements(comp_ary, Qsudoku);
            // console.log(comp_ary);

            if(sudoku.comp_ary_old === comp_ary){
                // implement this.
            } else {
                sudoku.comp_ary_old = comp_ary;
            }

            Qsudoku = self.FindElements(comp_ary, Qsudoku);
            // console.log(Qsudoku);

            if(self.IsThereNullElement(Qsudoku)){
                return Qsudoku;
            }

            if(i === 0){
                return null;
            }
        }
    }
};

And then you call it with this (Qsudoku being the value you want to pass in):

sudoku.DSSolve(Qsudoku);

Quick breakdown of changes:

changed all for loops and final while loop to decrement (faster in all browsers)
changed == '' to .length === 0 (faster in all browsers)
applied strict comparison === rather than implicit == (faster on certain browsers)
changed multiple if/else if/else statements to applying Math.floor to compute reduction factor
encapsulated all functions within object to allow for use of object comp_ary_old (instead of using window)
added explicit var statement for variable declaration (prevents bubbling up to window)
moved variables to top of respective function and assigned value at point where the fewest loops occur while retaining value integrity
changed the .toString() function to the +'' trick (its a miniscule improvement, more of a "squeeze every byte" thing, so if you would rather stick with code clarity switch it back to .toString())

I haven't tested this at all, so no benchmarks to show if it actually improves performance, but theoretically it should maintain your code operations while executing faster. Figured it was worth a shot, since no one else answered. Hope it helps!


Adding k to many elements in a sequence First line will contain two integers N and M separated by a single space. Then the next M lines will contain three integers a, b and k separated by a single space. N is the length of the sequence. M is the number of operations performed on that sequence. a is the starting index from a sequence and b is ending index from a sequence. I have to add k to all elements in between a and b. (I have to do this M times on N length sequence)

Here is my code:

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;

public class Solution {
    /**
     * @param args
     */
    public static void main(String[] args) {
        // TODO Auto-generated method stub
        Scanner sc = new Scanner(System.in);
        int n=sc.nextInt();
        int m=sc.nextInt();
        List l= new ArrayList(Collections.nCopies(n, 0));
        //System.out.println(l);

        for(int i=1;i<=m;i++){
            int a=sc.nextInt();
            int b=sc.nextInt();
            int k=sc.nextInt();
            for(int j=0;j<n;j++){
                //if((a-j)<=b){
                if(j+1>=a && j+1<=b){
                    int ab= (int) l.get(j);
                    int sum=ab+k;
                    l.set(j,sum);
                }
            }
        }
        int total=0;
        total=(int) l.get(0);
        for(int i=0;i<n;i++){
            int num=(int) l.get(i);
            if(num>total){
                total=num;
            }
        }
        System.out.println(total);
    }
}

Question: My code fails due to time out, and may be due to some other issues which i'm not aware of..please help to solve this or optimize my code.
It works for small inputs (100 lines)..

I guess the problem is in the inner loop:

for(int j=0;j<n;j++){
  if(j+1>=a && j+1<=b){
    //...
  }
}

If 'n' value is big and the gap between 'a' and 'b' is small enough iterations will be useless most of the time.
Also in my opinion, using of int[] array is more suitable here.

You can try in this way:

public final class Solution {

  public static void main(String[] args) {
    final Scanner scanner = new Scanner(System.in);
    final int n = scanner.nextInt();
    final int m = scanner.nextInt();
    final int[] l = new int[n];

    for (int i = 1; i <= Math.min(m, n - 1); i++) {
      final int a = Math.max(0, Math.min(scanner.nextInt() - 1, n - 1));
      final int b = Math.max(0, Math.min(scanner.nextInt(), n - 1));
      final int k = scanner.nextInt();
      for (int j = a; j < b; j++) { // iteration in useful range
        l[j] += k;
      }
    }
    int total = l[0];
    for (int i = 1; i < n; i++) {
      if (l[i] > total) {
        total = l[i];
      }
    }
    System.out.println(total);
  }
}


Changing background color of HTML elements based on user input This is jQuery code which works like this: depending on a user choice it changes the color and background of some elements in the DOM.

The behavior is ok, is doing exactly what I want.

And as you may see, there is some code repeated. What's the best way to keep a good performance by doing a function in order to avoid repeating code?

function ArticleQuiz($el){
    this.$el = $el;
    this.answer = this.$el.data('answer');
    this.answerContainer = this.$el.find('.answer-container');
    this.result = this.$el.find('.result');
    this.correctAnswer = this.$el.find('.correct-answer');
    this.incorrectAnswer = this.$el.find('.incorrect-answer');
    this.quizLogic();
    return this;
}

ArticleQuiz.prototype.quizLogic = function(){
    var THIS = this;
    var answer;

    $('.masthead-article-quiz').parent().css('background', '#eee');
    $('.quiz-cols a', this.$el).click(function(e) {
        THIS.target = $(e.target);
        e.preventDefault();
        // exit if choice already made: users can't change their pick
        if ($(this).parents('.quiz-cols').parent().find('.white-font').length) return;
        // set class according to data-answer:
        if (THIS.target.hasClass('icon-false-shape')) {
            THIS.answerContainer.show();
            $(this).addClass('background-' + ((THIS.answer === false) ? 'green' : 'red')).addClass('white-font');
            THIS.result.addClass('font-' + ((THIS.answer === false) ? 'green' : 'red'));
            answer =  THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();
        }
        if (THIS.target.hasClass('icon-true-shape')) {
            THIS.answerContainer.show();
            $(this).addClass('background-' + ((THIS.answer === true) ? 'green' : 'red')).addClass('white-font');
            THIS.result.addClass('font-' + ((THIS.answer === true) ? 'green' : 'red'));
            answer =  !THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();
        }
    });
    return THIS;
};

The first thing that jumps out at me is the tight coupling it has with the current application in which it is being used.  Why hard code all those CSS selectors?  This class really cries out for configuration.

You might see a pattern like this used to achieve decoupling of class from config.

function ArticleQuiz($el, $config){

   this.config = {};
   ...
   init() {
       $.extend(this.config, this.defaultConfig, $config || {});
       ...
   }
   this.init();
}

ArticleQuiz.defaultConfig = {
    answerDataLabel = 'answer',
    answerContainerSelector = '.answer-container',
    ...
}

You should consider taking the approach of using meaningful class names to apply multiple CSS styles at once.  So, rather than using specific semantic color and font class names, define a CSS class that encapsulates the entire state of being a correct/wrong answer.  So taking this approach along with the suggestion to store configuration something might allow you to change this:

        $(this).addClass('background-' + ((THIS.answer === false) ? 'green' : 'red')).addClass('white-font');
        THIS.result.addClass('font-' + ((THIS.answer === false) ? 'green' : 'red'));
        answer =  THIS.answer ? THIS.incorrectAnswer.show() : THIS.correctAnswer.show();

Into something like:

       var classToApply = THIS.config.correctAnswerClass;
       if(THIS.answer === true) {
           classToApply = THIS.config.incorrectAnswerClass;
       }
       $(this).addClass(classToApply).show();

Your QuizLogic function would not seem to have reason to be in the static (prototype) context, as you are using it as a constructor for your class. I would think this function should be within class function scope instead on prototype and should be call something like init() or similar common name for constructors in javascript.  Note how I present the init() function in my first code example above.

Consider first populating your instance properties in the constructor rather than in the main class code.  This gives you the ability to do things such as validate that the selectors do not return empty collections and similar activities needed to put the object in the proper state for use. If these validations fail, you could throw an error to prevent the object from being used in an improper state.


Shell-sort algorithm complexity I tried to implement the shell-sort algorithm using this Java code:

// Using Pratt sequence
public static void hSort(int tab[]) {
    int N = tab.length;
    int k = 0;
    int sequence = 1;

    // Getting the final term of the pratt sequence
    while(((int)(Math.pow(3, k) - 1) / 2) < N/3) {
        sequence = (int)(Math.pow(3, k) - 1) / 2;
        k++;
    }
    k--;

    while(sequence > 0) {
        hInsertionSort(tab, sequence);
        k--;
        sequence = (int)(Math.pow(3, k) - 1) / 2;
    }
}

// H sorting using insertion sort
public static void hInsertionSort(int[] tab, int h) {
    int N = tab.length;
    int k = 0;
    while (k < h) {
        for (int i = k; i < N; i+=h) {
            for (int j = i; j > k+h-1; j-=h) {
                if (less(tab[j], tab[j-h]) == -1) exch(tab, j, j-h);
                else break;
            }
        }
    k++;
    }
}

You can use the hsort method to try sort a table of integers.

Is the complexity of this implementation the same as the usual implementation?


Receiving MSMQ messages as a list I am unsure if the list could store a large number of messages.

There are 50,000 queue messages which I am receiving and assigning to the messages list:

    var msgEnumerator = msgQueue.GetMessageEnumerator2();
    var messages = new List<System.Messaging.Message>();
    while(msgEnumerator.MoveNext(new TimeSpan(0, 0, 1))) {
    var msg = msgQueue.ReceiveById(msgEnumerator.Current.Id, new TimeSpan(0, 0, 1));
    messages.Add(msg);
     }

    foreach( var k in messages){
    MailMessage mailM = (k.Body as SerializeableMailMessage).GetMailMessage();

    try {
    SmtpClient sp = new SmtpClient(smtpip, 25);
    sp.EnableSsl = false;
    sp.Send(mailM);
    }

    catch (Exception ex){
    logger.ErrorException("General error sending email.", ex);
    }
   }

Is this the correct way, or is there an alternative?  Let me hear your suggestions.

You create a lot of TimeSpan objects which is unnecessary. Just create one timeout variable. Especially since it's used in multiple places. This makes changing the timeout later easier. Otherwise you are bound to forget one place.
Prefer TimeSpan.FromXYZ() methods. They makes it more obvious for what period you create a TimeSpan.
Consider moving the mail sending code into its own function which just accepts a single message and sends it.
It might make sense to iterate over the message queue directly instead of buffering them all up:

var timeout = TimeSpan.FromSeconds(1);
var queueIter = msgQueue.GetMessageEnumerator2();
while (queueIter.MoveNext(timeout))
{
    using (var message = msgQueue.ReceiveById(queueIter.Current.Id, timeout))
    {
        SendMessage(message);
    }
}

Instead of directly sending the message consider making an event available on the class like MessageReceived. Then other code can subscribe to it and do stuff (like sending it as mail or logging it or saving it somewhere, etc).
As mentioned in the comment you should wrap all objects you create which are IDisposable into using statements to make sure any allocated resources are cleaned up properly.


Writing Read instances for HList I'm a total newbie at parsing, so I figured I'd start by writing instances of Read for HList, which does not currently have them. I'd appreciate some feedback on the following:

{-# LANGUAGE
  DataKinds
 ,KindSignatures
 ,FlexibleContexts
 ,FlexibleInstances
 ,TypeOperators
 ,GADTs
 ,ScopedTypeVariables
 #-}

module MyHList where
import GHC.Read
import Text.ParserCombinators.ReadPrec
import qualified Text.Read.Lex as L

-- This declaration is copied from `Data.HList.HList`.
data HList (l::[*]) where
  HNil  :: HList '[]
  HCons :: e -> HList l -> HList (e ': l)

infixr 2 `HCons`

instance Read (HList '[]) where
  readPrec = parens
    ( do
        expectP (L.Ident "H")
        expectP (L.Punc "[")
        expectP (L.Punc "]")
        return HNil
    )

instance forall e . Read e => Read (HList (e ': '[])) where
  readPrec = parens
    ( do
        expectP (L.Ident "H")
        expectP (L.Punc "[")
        e <- reset $ (readPrec::ReadPrec e)
        expectP (L.Punc "]")
        return (e `HCons` HNil)
    )

instance forall e f (l::[*]) . (Read e, ReadTl (HList (f ': l))) =>
   Read (HList (e ': f ': l)) where
  readPrec = parens
    ( do
        expectP (L.Ident "H")
        expectP (L.Punc "[")
        e <- reset $ (readPrec::ReadPrec e)
        expectP (L.Punc ",")
        rest <- readTl :: ReadPrec (HList (f ': l))
        return (e `HCons` rest)
    )

class ReadTl l where
  readTl :: ReadPrec l

instance forall e . Read e => ReadTl (HList (e ': '[])) where
  {-# INLINE readTl #-}
  readTl = do
    e <- reset $ (readPrec::ReadPrec e)
    expectP (L.Punc "]")
    return $ e `HCons` HNil

instance forall e f (l::[*]) . (Read e, ReadTl (HList (f ': l)))
             => ReadTl (HList (e ': f ': l)) where
  {-# INLINE readTl #-}
  readTl = do
    e <- reset $ (readPrec::ReadPrec e)
    expectP (L.Punc ",")
    rest <- readTl
    return (e `HCons` rest)

This is indeed interesting challenge!

-- Read after H[
class ReadHList (a :: [*]) where
  readHList :: ReadPrec (HList a)

-- Read after element:
-- ] for nil
-- , readHList for cons
class ReadHListAfter (a :: [*]) where
  readHListAfter :: ReadPrec (HList a)

instance ReadHList '[] where
  readHList = expectP (L.Punc "]") >> return HNil

instance ReadHListAfter '[] where
  readHListAfter = readHList

instance (Read a, ReadHListAfter l) => ReadHList (a ': l) where
  readHList = ...

-- We could use ReadHList (a ': l) as additional constraint,
-- But then we need UndecidableInstances extension
instance (Read a, ReadHListAfter l) => ReadHListAfter (a ': l) where
  readHListAfter = ...

-- Parsing HList is trivial as composing functions!
instance ReadHList a => Read (HList a) where
  readPrec = parens $ do
    expectP (L.Ident "H")
    expectP (L.Punc "[")
    readHList

This way it's a bit more elegant. IMHO.

Compare it to pseudo-codish parsing of lists of known length (aka vectors):

list :: Int -> Parser a -> Parser [a]
list n p =
  token '['
  list' n p

list' :: Int -> Parser a -> Parser [a]
list' 0 _ = token ']' >> return []
list' n p = do
  h <- p
  t <- listTail (n - 1) p
  return (h : t)

listTail :: Int -> Parser a -> Parser [a]
listTail 0 _ = token ']' >> return []
listTail n = do
  h <- p
  token ','
  t <- listTail (n - 1) p
  return (h : t)