Is this correct way to use cookies from a syntax point of view? I'd like to know if this is the right syntax for using cookies. I haven't yet tested this code with cookies being used.

I'm using a jQuery Plugin called jPages to display pages on my (one page) website, it has a lot of options and it's very flexible.

The jPages plugin has an option to start from a different page per say, and a callback function to find what page you are currently on.

I'm using localStorage to store the last page that a visitor has been on so that it can be retrieved when they return. I've tested this method and it works perfectly.

I'm using Modernizr to test for localStorage support in browsers, and if localStorage is not supported I use cookies to try and achieve the same experience.

Here's the part of my code that does that:

var set_data = function(cluster) {
    if (!localStorage.getItem("Page")) {
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    } else {
        localStorage.removeItem("Page");
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    }
}

var get_data = function() {
    if (localStorage.getItem("Page")) {
        var last_page = JSON.parse(localStorage.getItem("Page"))
        log("Last Page: " + last_page);
        return last_page;
    } else {
        log("Last Page: Inexistent");
        return 1;
    }
}

var set_last_page = function(cluster) {
    if (Modernizr.localstorage) {
        var data = cluster;
        set_data(data);
    } else {
        var data = JSON.stringify(cluster);
        create_cookie("Page", data, 365);
    }
}

var get_current_page = function() {
    if (Modernizr.localstorage) {
        var data = get_data();
    } else {
        var data = JSON.parse(read_cookie("Page"));
    }
    return data;
}

var create_cookie = function(name, value, days) {
    var date;
    var expires;

    if (days) {
        date = new Date();
        date.setTime(date.getTime() + (days * 24 * 60 * 60 * 1000));
        expires = "; expires=" + date.toGMTString();
    } else {
        expires = "";
    }
    document.cookie = name + "=" + value + expires + "; path=/";
}

var read_cookie = function(name) {
    var nameEQ = name + "=";
    var ca = document.cookie.split(';');
    var i;
    var c;

    for (i = 0; i < ca.length; i++) {
        c = ca[i];
        while (c.charAt(0) == ' ') {
            c = c.substring(1, c.length);
        }

        if (c.indexOf(nameEQ) == 0) {
            return c.substring(nameEQ.length, c.length);
        }
    }
    return null;
}

The localStorage part works, but I'm not sure if I'm doing the cookie part correctly.

If someone can tell me if this is the right syntax for using cookies, I would appreciate it.

At the expense of having a little more code, I would use a localStorage polyfill [2].

With that code in place you no longer need to keep the create and read cookie methods. You also don't need the code wrapping get_data and set_data. Your resulting code would look something like this:

set_last_page = function(cluster) {
    if (!localStorage.getItem("Page")) {
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    } else {
        localStorage.removeItem("Page");
        if (!localStorage.setItem("Page", JSON.stringify(cluster))) {
            log("Current Page: Stored");
        } else {
            log("Current Page: Store Failed");
        }
    }
}

get_current_page = function() {
    if (localStorage.getItem("Page")) {
        var last_page = JSON.parse(localStorage.getItem("Page"))
        log("Last Page: " + last_page);
        return last_page;
    } else {
        log("Last Page: Inexistent");
        return 1;
    }
}

From here I would make a few changes:

You don't need to check for existence or remove an item from storage to change it.
The logging code is unnecessary.
Using JSON.stringify and JSON.parse is considerable overkill for converting between a string and a number.

Taking care of these would leave me with this code:

set_last_page = function(cluster) {
    localStorage.setItem("Page", cluster+'');
}

get_current_page = function() {
    return +(localStorage.getItem("Page") || 1);
    //localStorage.getItem("Page") will return either
    // a number >= 1 (as a string), undefined or null
    //so '|| 1' will take care of the latter cases
}

Finally, I dislike the fact you are initializing these variables as function expressions (statements) and don't end them with semicolons (either add the semis or make them function declarations). I would also rename things to be more consistent and in line with my preferred naming conventions for javascript:

function setPage(page) {
    localStorage.setItem('page', page+'');
}

function getPage() {
    return +(localStorage.getItem('page') || 1);
}


Have I gone too far in factoring out repeated code in this example? In a controller (PHP, CodeIgniter) I'm adding scripts & css to variables to be used in the view output. My initial coding looked like this:

private function _add_script($path, $where = self::ADD_LAST) {
    $html = "<script src='$path'></script>\n";
    switch($where) {
        case self::ADD_LAST:
            array_push($this->_data['script'], $html);
            break;
        case self::ADD_FIRST:
            array_unshift($this->_data['script'], $html);
            break;
    }
}

private function _add_css($path, self::ADD_LAST) {
    $html = "    <link rel='stylesheet' href='$path'>\n";
    switch($where) {
        case self::ADD_LAST:
            array_push($this->_data['css'], $html);
            break;
        case self::ADD_FIRST:
            array_unshift($this->_data['css'], $html);
            break;
    }
}

Well, I noticed that these two are basically just repeating themselves, so I immediately rewrote them into this format:

private function _add_script($path, $where = self::ADD_LAST) {
    $html = "<script src='$path'></script>\n";
    $this->_add_external('script', $html, $where);
}

private function _add_css($path, $where = self::ADD_LAST) {
    $html = "<link rel='stylesheet' href='$path'>\n";
    $this->_add_external('css', $html, $where);
}

private function _add_external($type, $html, $where) {
    switch($where) {
        case self::ADD_LAST:
            array_push($this->_data[$type], $html);
            break;
        case self::ADD_FIRST:
            array_unshift($this->_data[$type], $html);
            break;
    }
}

Does the second version seem like the way to go? Or perhaps a 3rd alternative. The requirements call for being able to add elements to the list either before or after all other elements already in the list.

I think what you did was a good thing. I also think you could combine the first 2 functions as well.

Something like this:

private function _add_item($path, $type, $where = self::ADD_LAST) {
    $script = "<script src='$path'></script>\n";
    $css = "<link rel='stylesheet' href='$path'>\n";

    if ($type == 'script') {
        $html = $script;
    }
    elseif ($type == 'css') {
        $html = $css;
    }

    $this->_add_external($type, $html, $where);
}

Or this:

private function _add_item($path, $type, $where = self::ADD_LAST) {
    $script = "<script src='$path'></script>\n";
    $css = "<link rel='stylesheet' href='$path'>\n";

    switch($type) {
        case 'script':
            $html = $script;
            break;
        case 'type':
            $html = $css;
            break;
    }

    $this->_add_external($type, $html, $where);
}

Or this (My favorite):

private function _add_item($path, $type, $where = self::ADD_LAST) {
    $array['script'] = "<script src='$path'></script>\n";
    $array['css'] = "<link rel='stylesheet' href='$path'>\n";

    $html = $array[$type];

    $this->_add_external($type, $html, $where);
}

EDIT:

And I think this would work too, an all in one solution:

private function _add_item_external($path, $type, $where = self::ADD_LAST) {
    $array['script'] = "<script src='$path'></script>\n";
    $array['css'] = "<link rel='stylesheet' href='$path'>\n";

    switch($where) {
        case self::ADD_LAST:
            array_push($this->_data[$type], $array[$type]);
            break;
        case self::ADD_FIRST:
            array_unshift($this->_data[$type], $array[$type]);
            break;
    }
}


Unit test Web API controller that uses a static helper class which uses app config setting I have a situation where I have a static class that reads my application configuration file for some configuration value. When unit testing Web API controllers that make use of this static class I am getting an error about the confgiration setting being null.

I could have duplicated the value of the key in my app config for the unit test project but that seemed dirty.

Instead I have tried to abstract the dependency on app settings via a configuration interface and inject the interface using Unity. This all seems to work but I've no idea if I have done this correctly (code below).

NB: I am using FakeItEasy for mocking in unit tests.

So my question is simple - is this the correct way?

Interface for accessing App config

public interface IConfigurationReader
{
    string GetAppSetting(string key);
}

public class ConfigurationReader : IConfigurationReader
{
    public string GetAppSetting(string key)
    {
        return ConfigurationManager.AppSettings[key];
    }
}

Add init method to static class

private static IConfigurationReader _config;

public static void Init(IConfigurationReader config)
{
    _config = config;
}

Register & resolve in Unity Register Types method

unityContainer.RegisterType<IConfigurationReader, ConfigurationReader>("Config");
HeaderHelper.Init(unityContainer.Resolve<IConfigurationReader>("Config"));

In my unit test project fake interface and call static init

_config = A.Fake<IConfigurationReader>();
HeaderHelper.Init(_config);

In my unit test fake call to GetAppSetting

A.CallTo(() => _config.GetAppSetting("AES")).Returns(AesKey);

You're doing it wrong.

You can't do Dependency Injection and have static class dependencies scattered throughout your code: when you do Dependency Injection, you need to go all the way or not at all, else you gain the complexity and lose the benefits.

Proper DI goes like this:

Register: Set up your IoC container and register dependencies.
Resolve: Resolve the entire application's dependency graph - this is ONE method call.
Release: Dispose any disposables, clean up and tear down.

Your solution doesn't follow the Resolve step correctly, since you're calling .Resolve to fetch a single specific type, only to call that specific Init method on your static class: in all likelihood you're eventually going to need to do this in more than one place, and that's only because you stopped injecting dependencies at one point.

Any code that uses this HeaderHelper static class has a hidden dependency that isn't being injected, which means every single piece of code that uses HeaderHelper is tightly coupled with one specific implementation of that class.

Consider the below hypothetical MyClass; its constructor implicitly documents the type's dependencies (here ISomeDependency), but DoSomething has a dependency that isn't injected from the outside:

private readonly ISomeDependency _dependency;

public MyClass(ISomeDependency dependency)
{
    _dependency = dependency; // explicit dependency, constructor-injected
}

private void DoSomething()
{
    var foo = HeaderHelper.Foo; // HeaderHelper is a hidden dependency
    _dependency.DoSomething(foo);
}

There are several ways to fix this, but which is best for your case largely depends on context you haven't provided in your question (we know nothing of HeaderHelper other than it has a vague useless name that doesn't mean anything - you should really avoid "Helper" in class names).

1. Constructor Injection

You could do exactly like you did with the static ConfigurationManager class, and wrap it with an interface:

public interface IHeaderHelper // todo: rename!!!
{
    // members
}

That Init method seems rather artificial, and wouldn't be needed if the class' constructor took an IConfigurationReader dependency.

public class HeaderHelper : IHeaderHelper // notice: NOT static
{
    private readonly IConfigurationReader _configReader;

    public HeaderHelper(IConfigurationReader configReader)
    {
        _configReader = configReader;
    }

    // members
}

And then every class that needs to use a HeaderHelper should intake an IHeaderHelper constructor argument, and replace the implicit dependencies to the static class with explicit dependencies to that interface, so instead of this:

HeaderHelper.Foobar();

You would have that:

_headerHelper.Foobar();

Where _headerHelper is a private readonly IHeaderHelper instance field.

2. Ambient Context

Constructor injection is nice, but sometimes there are cross-cutting dependencies that are needed pretty much everywhere - it's fairly possible that configuration is one of such dependencies. Wrapping it with an interface and constructor-injecting it will work, but then you'll end up with lots of classes that intake this dependency, and when that feels like constructor-bloating, you might want to explore other possibilities.

Mark Seemann's blog describes an interesting pattern to address this problem: the Ambient Context.

This DI pattern would change this:

var foo = HeaderHelper.Foo();

Into this:

var foo = HeaderSettingsContext.Current.Foo();

The Ambient Context is still an implicit dependency though, and Constructor Injection should be preferred most of the time. But by storing the current context in thread-local storage, your tests can easily manipulate the current context, and alter what Foo() does in your application (e.g. hit the file system and read some XML value vs return a mocked-up value).

This article by Adras Nemes describes more DI patterns (the article is based on Mark Seemann's work).


Find end of $.each() loop I need to find the end of a loop or the length of the object. Some context on the code:

I obtain the txt var from the DOM (that's the only way I've got by now and can't change it) and it's a formatted JSON.  Here's an example of the expected JSON output:

  {"SE DIO AVISO A CENTRAL":true,"OTRO":false,"SE DIO AVISO A SEGURIDAD
  CIUDADANA":true,"SIN ACCIÓN":false,"SE DIO AVISO A
  AMBULANCIA":false,"SE DIO AVISO A CARABINEROS":true,"SE DIO AVISO A
  BOMBEROS":false}

My idea here is to only show the ones with value trueone after the other followed by a comma, but changing that at the end of the loop for a dot. For the moment I've got the commas down but not the dot, which is why I need to know either the length of obj or the end of the $.each() loop. Any improvements on overall code are appreciated.

try{
    var obj=jQuery.parseJSON(txt);
    $(self).text(" ");
    var i = 0;
    jQuery.each(obj, function(text, val){
        if(val){
            if(i == 0){
                $(self).append(text);
            }else{
                $(self).append(", "+text);
            }
        }
        i++;
    });
}
catch(e){
}

You can do this with jQuery.map extremely elegantly as the function allows you to do a filtered map (.map excludes any item when you return null or undefined). As $.map produces an array you can then just use Array.prototype.join and get the same output as our original code. If you want to add a period to the string after that then you can check if your string isn't empty, append a period and be on your way.

var text = $.map(obj, function(val, key) { if(val) return key; } ).join(", "); //Create an array of an objects keys where its value is truthy. Next, join the keys.
if(text !== "") text += "."; // Append a period if not an empty string

$(self).append(text);

Also you should reconsider using a try catch block around the JSON parsing as you shouldn't expect that to fail without good reason.


LeetCode: N-ary Tree Level Order Traversal https://leetcode.com/problems/n-ary-tree-level-order-traversal/

Given an n-ary tree, return the level order traversal of its nodes' values. (ie, from left to right, level by level).

For example, given a 3-ary tree:

We should return its level order traversal:

[
     [1],
     [3,2,4],
     [5,6]
]

Note:

The depth of the tree is at most 1000.
The total number of nodes is at most 5000.

Please comment about space and time complexity.
Thanks.

using System.Collections.Generic;
using System.Linq;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace GraphsQuestions
{
    /// <summary>
    /// https://leetcode.com/problems/n-ary-tree-level-order-traversal/
    /// </summary>

    [TestClass]
    public class N_aryTreeLevelOrderTraversal
    {
        [TestMethod]
        public void N_aryTreeLevelOrderTraversalTest()
        {
            List<Node> node3 = new List<Node>();
            node3.Add(new Node(5, null));
            node3.Add(new Node(6, null));

            List<Node> node1 = new List<Node>();
            node1.Add(new Node(3, node3));
            node1.Add(new Node(2, null));
            node1.Add(new Node(4, null));

            Node root = new Node(1, node1);
            var result = LevelOrder(root);
            IList<IList<int>> expected = new List<IList<int>>();
            expected.Add(new List<int>{1});
            expected.Add(new List<int>{3,2,4});
            expected.Add(new List<int>{5,6});

            for (int i = 0; i < 3; i++)
            {
                CollectionAssert.AreEqual(expected[i].ToArray(), result[i].ToArray());
            }

        }
        public IList<IList<int>> LevelOrder(Node root)
        {
            IList<IList<int>> result = new List<IList<int>>();
            Queue<Node> Q = new Queue<Node>();
            if (root == null)
            {
                return result;
            }

            Q.Enqueue(root);

            while (Q.Count > 0)
            {
                List<int> currentLevel = new List<int>();
                int qSize = Q.Count;
                for (int i = 0; i < qSize; i++)
                {
                    var curr = Q.Dequeue();
                    currentLevel.Add(curr.val);
                    if (curr.children != null)
                    {
                        foreach (var child in curr.children)
                        {
                            Q.Enqueue(child);
                        }
                    }
                }
                result.Add(currentLevel);
            }
            return result;
        }
    }
}

Time and space complexity are just the number of nodes in the graph. You can't do better than this with the API given. If the API were open to negotiation, I'd consider IEnumerable<IReadonlyList<int>>, which would permit better memory characteristics for some trees (e.g. if the level-width stays reasonably constant rather than, say, growing exponentially). There is also no need for this to be an instance method.

I don't like this:

if (root == null)
{
    return result;
}

No-where in the spec is it said that a null input should produce a null output, and this is a design decision which needs to be documented. Without something saying that null should map to null, I'd much prefer an ArgumentNullException which doesn't obscure any assumptions. If this behaviour is wrong, then you'll know the moment you try to use it. If returning null is wrong, it might take you a while to find out where your code messed up. Either behaviour warrents inline documentation. I'd also prefer these sorts of checks were performed straight-away, so that they can't be lost in the other clutter, and in this case avoid unnecessarily allocations.

Similarly, is it specified that Node.children can be null, and that this is supported? If not, then I'd be inclined to remove the check, as it will simplify the code and cause it to blow up if this assumption is violated, forcing the consumer to address the null rather than hoping the behaviour is as expected. (Ideally any constraint would be enforced by the Node class, and I appreciate that isn't yours to change).

Q isn't a great variable name: it describes what the thing is, which can be confused with its purpose; and because it is capitalised, it feels like a member variable.

It would be nicer if the test methods were separated from the functionality. Currently, anyone trying to find aryTreeLevelOrderTraversal.LevelOrder will be confronted with N_aryTreeLevelOrderTraversalTest in their intellisense

Personally I don't like this sort of 'staged' consumption of a queue; it just feels fiddily. In this case you can avoid it completely (and save memory) by enumerating over the previous level when you construct the next. This can make the code more-compact, and harder to break (e.g. someone might 'helpfully' inline qSize thinking it's clearer only to break the code completely) with fewer variables whereof to keep track. With a bit of LINQ, we can write (untested):

List<int> level = new List<int> { root };
while (level.Count > 0)
{
    result.Add(level);
    level = level.Where(n => n.Children != null).SelectMany(n => n.children).ToList();
}

There is virtually nothing to go wrong here apart from getting those 2 lines inside the loop the wrong way round. You don't need anything more complex unless performance is a real concern, or you want a lazy implementation, in which case you need to be careful you don't yield state you are about to access:

List<int> level = new List<int> { root };
while (level.Count > 0)
{
    var previous = level;
    level = previous.Where(n => n.Children != null).SelectMany(n => n.children).ToList();
    yield return previous;
}


Mockup in C of hardware implementation for multiplication of signed integers I challenged myself to implement multiplication of signed integers using only +, -, >>, <<, &, |, ^, and conditionals, though I would like to improve my method to not use conditionals if possible in the future. Any input would be very helpful.

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <limits.h>
#include <time.h>

int multiply(int, int);

int main() {
    int a, b, c, i;
    bool equal;

    srand(time(NULL));

    for (i = 0; i < 100; i++) {
        // 0x8000 to ensure no integer overflow is possible
        a = (rand() % 0x8000) * (rand() % 2 ? -1 : 1);
        b = (rand() % 0x8000) * (rand() % 2 ? -1 : 1);
        c = multiply(a, b);
        equal = (c == a * b);

        printf("%-11d * %-11d = %-11d (%s)\n", a, b, c, (equal ? "Success" : "Failure"));

        if (!equal) {
            return EXIT_FAILURE;
        }
    }

    return EXIT_SUCCESS;
}

int multiply(int a, int b) {
    struct {
        unsigned char index : 5;
    } bit;

    int c = INT_MAX & a, // remove sign bit
        d = INT_MAX & b,
        sign = (INT_MIN & a) ^ (INT_MIN & b), // sign of product
        mask = 0x40000000,
        prod = 0;

    bit.index = 30; // keeping track of mask bit index

    do {
        prod += (mask & c ? d << bit.index : 0);
    } while ((bit.index--) && (mask >>= 1));

    return sign | (prod & INT_MAX);
}

Sign bit manipulation unnecessary

I looked at your code and was puzzled by all the sign bit manipulation.  As far as I can tell, it's not necessary.  I rewrote your function to be like the following and it still passed all your tests:

int multiply(int a, int b)
{
    // Bit keeps track of mask bit index.
    struct {
        unsigned int index: 5;
    } bit = {31};
    int prod = 0;

    do {
        prod += ((a & (1 << bit.index)) ? b << bit.index : 0);
    } while (bit.index--);

    return prod;
}

Bonus answer: no conditionals inside loop

int multiply(int a, int b)
{
    // Bit keeps track of mask bit index.
    struct {
        unsigned int index: 5;
    } bit = {31};
    int prod = 0;

    do {
        prod += (-((a >> bit.index) & 1)) & (b << bit.index);
    } while (bit.index--);

    return prod;
}

Explanation:

((a >> bit.index) & 1) is 1 or 0 depending on the masked bit.  Call this B.
-B is -1 or 0.  This sets up a mask to use for the next step (-1 is a mask of all one bits).
-B & (b << bit.index) is either (b << bit.index) or 0.


Let's protect that CLI Python script I needed a way to make a CLI based script a bit secure. So, I thought of the following workflow:

the client will get the MAC of the user, sha256 it and put it in a file at a specific location
then in my program, I'm also doing the same thing, check to see if the sha256 of the MAC is present in the file (and if it matches) then let the user use the script.
more, that text file will have multiple lines in it (but the only one that maters is the first one)

I'm aware that this might not be the best option for securing a script, but the client said ("do this to make it a bit harder for somebody else to use it").

import hashlib
import sys

from uuid import getnode

def get_mac():
    """return the MAC address of a specific machine"""
    return bytes(str(getnode()), 'utf-8')

def hash_mac():
    """hash the MAC using sha256 algorithm"""
    return hashlib.sha256(get_mac()).hexdigest()

def main():
    with open('uid_file.cfg') as uid_file:
        first_line = uid_file.readline().replace('\n', '')

        if str(hash_mac()) in first_line and len(str(hash_mac())) == len(first_line):
            print('Match MAC and execute program here\n')
        else:
            print('MAC does not match! Exiting...')
            sys.exit(0)

if __name__ == '__main__':
    main()

Any advice on what could go wrong or alternatives / good practices are welcome.

hexdigest returns a string

So you don't need to str(hash_mac()) each time. Even if it didn't, you would probably want to put the str call within that hash_mac function.
You're not using get_mac for any other purpose than hashing it.

You might thus want to encapsulate this functionality into hash_mac:

from uuid import getnode as get_mac

def hash_mac():
    """hash the MAC Address using sha256 algorithm"""
    mac = str(get_mac()).encode()
    return hashlib.sha256(mac).hexdigest()

You don't need to get a file open for that long.

You can close it as soon as you read its first line. As a bonus, you can also directly compare strings:

def main():
    with open('uid_file.cfg') as uid_file:
        first_line = uid_file.readline().strip()

    if hash_mac() == first_line:
        print('Match MAC and execute program here\n')
    else:
        print('MAC does not match! Exiting...')
        sys.exit(0)

You can feed error messages to sys.exit:

sys.exit('Mac does not match! Exiting...')

will print Mac does not match! Exiting... to stderr and exit the program with a status code of 1. Note that usually, a status code of 0 means success.
If you want to use it as a form of security, you should avoid giving hints of what to do to unauthorized persons:

if uid_file.cfg does not exist, a traceback saying it tried to open it will be printed
if content of the file mismatch, a hint is given such that it should contain a MAC address

You can try to avoid that with a generic error message such as Access denied:

def main():
    try:
        with open('uid_file.cfg') as uid_file:
            first_line = uid_file.readline().strip()
    except OSError:
        sys.exit('Access denied')

    if hash_mac() != first_line:
        sys.exit('Access denied')

    print('Match MAC and execute program here\n')

Lastly, it might be interesting to define this function as a check that you can easily disable while developping/testing:

import sys
import hashlib
from uuid import getnode as get_mac

def hash_mac():
    """hash the MAC Address using sha256 algorithm"""
    mac = str(get_mac()).encode()
    return hashlib.sha256(mac).hexdigest()

def check_secure_user():
    try:
        with open('uid_file.cfg') as uid_file:
            first_line = uid_file.readline().strip()
    except OSError:
        sys.exit('Access denied')

    if hash_mac() != first_line:
        sys.exit('Access denied')

def main():
    print('Match MAC and execute program here\n')

if __name__ == '__main__':
    check_secure_user()  # deactivate while developping
    main()


Can this localStorage login script be written more efficiently? I understand that account details should be stored using a much more secure method, but this is only a demonstration script I've made to store login credentials and remember a user wishes to remain logged in using localStorage rather than sessionStorage.

Could anyone show me how I might write this more efficiently please? My main concern is code repetition, so I'd love to move toward a much more object oriented structure.

var bic = bic || {};

bic.checkLogin = function() {
    var loginUser = document.getElementById("loginUser").value;
    var loginPass = document.getElementById("loginPass").value;
    var storedUser = localStorage.Username;
    var storedUser2 = storedUser.replace(/"/g,''); // remove " for plain text display in loginForm
    var storedPass = localStorage.Password;
    var checked = document.getElementById("rememberLogin").checked;

    if(checked == true) {
        localStorage.setItem("loggedIn", "yes");
    } else {
        localStorage.setItem("loggedIn", "no");
    }

    // if Username key exists, and user/pass values match storage entries
    if(localStorage.Username) {
        if(storedUser.match(loginUser) && storedPass.match(loginPass)) {
            bic.notify("loginForm", "Logged in as: " + storedUser2 + "

Log out?");
            document.getElementById("logout").onclick = function(e){e.preventDefault();bic.logout();};
            console.log("Logged in as: " + localStorage.Username);
        } else {
            bic.loginFailed();
            bic.notify("loginStatus", "Login failed. Please try again.");
            console.log("Failed logins: " + localStorage.failedLogins);
        }
    }
}

// localStorage equiv of cookie
bic.isLoggedIn = function() {
    var a = localStorage.loggedIn;
    var storedUser = localStorage.Username;
    var storedUser2 = storedUser.replace(/"/g,''); // removing " as above

    // does a exactly equal "yes"?
    if(a === "yes") {
        bic.notify("loginForm", "Logged in as: " + storedUser2 + "

Log out?");
        document.getElementById("logout").onclick = function(e){e.preventDefault();bic.logout();};
        console.log("Logged in as: " + localStorage.Username);
    }
}

// logout and remove localStorage key
bic.logout = function() {
    document.getElementById("loginForm").innerHTML = "<input type='text' id='loginUser' placeholder='Username'>

<input type='password' id='loginPass' placeholder='Password'>

<button type='button' id='login'>Login</button> <button type='button' id='loginReg'>Register</button>

<input type='checkbox' id='rememberLogin' /> Stay logged in?
";
    bic.loginPrefill();
    bic.delete("loggedIn");
    console.log("Logged out");
}

Separate UI from logic

You might want to keep the logic for localStorage handling separate from the DOM fetching logic. That way, your system for handling the local storage can easily be detached and reused elsewhere without that extra baggage. Also works the other way around, when you replace your UI.

Basic login API

In continuation with the previous paragraph, you could model it to have this API:

bic.login(username,password);   // Return true if logged in, false if not
bic.isLoggedIn();               // Return true if someone is logged in, else false
bic.getUserId();                // Gets the current user's ID
bic.logout();                   // Logout current user

bic.register(username,password) // Registration
bic.destroyCurrentUser();       // Just for fun. Only a logged in user can delete himself

The UI logic is separate. It consumes this API rather than being one with it.

LocalStorage is synchronous

Note that localStorage is a synchronous operation. Doing heavy IO against localStorage could mean an unresponsive experience. This has an amplified effect on mobile devices, since they are times slower. Don't pull out or put in huge data. Do it by chunks. Use multiple small keys rather than one big key to house everything your system needs.

JSON.parse() and JSON.stringify()

localStorage is a string-based, key-value storage. But that's it. You need some form of format to structure highly complex data, like user data. Something that may look like:

[
  {id:1,name:"foo",password:"***"},
  {id:1,name:"bar",password:"***"},
  {id:1,name:"baz",password:"***"}
]

To store data like a JS object, you need JSON.stringify() to turn it into a string. Then you can store it to localStorage. To get it, retrieve it from localStorage and then use JSON.parse() to convert from string.

Storage routine

What I typically do when using localStorage is to create an in-memory object of the data. On every save, I serialize the data with JSON.stringify and store it to the localStorage. If ever there are other code that modifies that same data, there are localStorage events to tell you if the data is modified in the storage.

Code refinements

Since I advertised array-based storage, then Array filter, splice, push would be your best friends here. In addition, I believe I introduced you to JSON stringify and parse already.


User-like MongoDB implementation This is my first implementation of MongoDB, much much more to come. This sheds light on some things like how I've structured my collection. I hope to gain feedback and insight from the community on best practices as well as do's and don'ts.

Givens

I have a website with multiple apps.
Each user can at one point signup / login for an app.
There is one MongoDB collection called users that will store all the information.
I want a log for each app that tracks users actions (signup, login, logout).

Example User Document

{
  "_id": ObjectId("5035b9fc0000005eed000002"),
  "apps": {
    "app0": {
      "log": [
        {
          "status": "signup",
          "created_at": ISODate("2012-08-23T05:05:00.872Z")
        },
        {
          "status": "login",
          "created_at": ISODate("2012-08-23T05:05:04.012Z")
        },
        {
          "status": "login",
          "created_at": ISODate("2012-08-23T05:05:06.236Z")
        }
      ]
    }
  },
  "user": "reggi"
}

This code is the first interaction with the user they put a small piece of information in and I have to determine whether they are already in the database or not. They can initiate the signup process more than once, so both the initial and update need to write the same log.

users.findOne({"username":req.query.user},function(err, post){
  if(typeof(post) == "undefined"){
    //username does not exist in users create it with this app instance
    users.save((function(){
      var object = {
        "username":req.query.user,
        "apps":{},
      };
      object.apps[appAlias] = {};
      object.apps[appAlias].log = [];
      object.apps[appAlias].log.push({
        status: "initiated-signup",
        created_at: new Date(),
      });
      return object;
    }()));
  }else{
    //username does exist in users create new apps object
    users.update({"_id":post._id},(function(){
      var object = {};
      object.$push = {};
      object.$push["apps."+appAlias+".log"] = {
        status: "initiated-signup",
        created_at: new Date()
      };
      return object;
    })())
  }
});

This is my first dabble with MongoDB. I'm using Mongolian.

While I cannot comment on your Node.js code, I can tell you that we're doing something very similar on a project that uses Mongo to store every bit of information for around 200 social profiles. In our case, we're doing something like:

foreach ($videos as $video) {
    $payload = ['$set'  => $defaults + $meta + $this->interjectTimestamps((array)$video)];

    if (isset($video['statistics'])) {
        // Maintain a log of statistics for each video
        $payload['$push'] = ['statistics_log' => ['execute_at_ts' => $meta['execute_at_ts'], 'statistics' => $video['statistics']]];
    }
    $coll->update(['_type' => 'video', 'id' => $video['id']], $payload, ['upsert' => true]);
}

From my research, this is fine. Mongo's $push works great for logs like this, but be wary, it can be difficult to query data in array's like this. If you plan on doing a lot of querying against a user's app's, it may be better to store this information in a separate collection.

On the flip side, this design can be crucial if you ever need to perform any calculations with data from both your User document and the nested log. In our case, we're doing MapReduce on data from the entire document, log and all.


Moving window with complete boundary in Python I asked this question a few days ago. The question is about producing moving/rolling windows of an array in Python as in the following example:

from skimage.util import view_as_windows
view_as_windows(np.arange(10*10).reshape(10,10), (4,4),4)

Out[14]:
array([[[[ 0,  1,  2,  3],
         [10, 11, 12, 13],
         [20, 21, 22, 23],
         [30, 31, 32, 33]],

        [[ 4,  5,  6,  7],
         [14, 15, 16, 17],
         [24, 25, 26, 27],
         [34, 35, 36, 37]]],

       [[[40, 41, 42, 43],
         [50, 51, 52, 53],
         [60, 61, 62, 63],
         [70, 71, 72, 73]],

        [[44, 45, 46, 47],
         [54, 55, 56, 57],
         [64, 65, 66, 67],
         [74, 75, 76, 77]]]]).

However, this removes elements of the original arrays that don't fit into another window. It was suggested that I should fill the arrays with zeros in order to get a full window, so I wrote the following code for doing so and I thought it would be nice to have a review in case I missed corner cases:

import numpy as np

def extend_image(image, steps=16):
    # Required number of rows
    required_rows = np.mod(image.shape[0], steps)
    # Required number of columns
    required_cols = np.mod(image.shape[1], steps)
    # Concatenate original array with rows filled with zeros
    partial = np.concatenate((image, np.zeros((required_rows,image.shape[1]))))
    # Concatenate partial array with columns filled with zeros
    extended = np.concatenate((partial, np.zeros((partial.shape[0], required_cols))), axis=1)
    return extended

This should work as follows:

view_as_windows(extend_image(np.arange(10*10).reshape(10,10), steps=4), (4,4),4)

array([[[[  0.,   1.,   2.,   3.],
         [ 10.,  11.,  12.,  13.],
         [ 20.,  21.,  22.,  23.],
         [ 30.,  31.,  32.,  33.]],

        [[  4.,   5.,   6.,   7.],
         [ 14.,  15.,  16.,  17.],
         [ 24.,  25.,  26.,  27.],
         [ 34.,  35.,  36.,  37.]],

        [[  8.,   9.,   0.,   0.],
         [ 18.,  19.,   0.,   0.],
         [ 28.,  29.,   0.,   0.],
         [ 38.,  39.,   0.,   0.]]],

       [[[ 40.,  41.,  42.,  43.],
         [ 50.,  51.,  52.,  53.],
         [ 60.,  61.,  62.,  63.],
         [ 70.,  71.,  72.,  73.]],

        [[ 44.,  45.,  46.,  47.],
         [ 54.,  55.,  56.,  57.],
         [ 64.,  65.,  66.,  67.],
         [ 74.,  75.,  76.,  77.]],

        [[ 48.,  49.,   0.,   0.],
         [ 58.,  59.,   0.,   0.],
         [ 68.,  69.,   0.,   0.],
         [ 78.,  79.,   0.,   0.]]],

       [[[ 80.,  81.,  82.,  83.],
         [ 90.,  91.,  92.,  93.],
         [  0.,   0.,   0.,   0.],
         [  0.,   0.,   0.,   0.]],

        [[ 84.,  85.,  86.,  87.],
         [ 94.,  95.,  96.,  97.],
         [  0.,   0.,   0.,   0.],
         [  0.,   0.,   0.,   0.]],

        [[ 88.,  89.,   0.,   0.],
         [ 98.,  99.,   0.,   0.],
         [  0.,   0.,   0.,   0.],
         [  0.,   0.,   0.,   0.]]]])


4-Bit Ripple Carry Adder in Haskell I recently started learning Haskell and had to program a 4 Bit Ripple Carry Adder that worked with tuples of the form

type Nibble = (Bool, Bool, Bool, Bool)

Beforehand I wrote the functions

-- XOR operator
xor :: Bool -> Bool -> Bool
xor a b = (not (a && b) ) && (a || b)

-- Fulladder for 1-Bit binary numbers
-- Returns Tuple in the Form (add Carry, Sum)
fulladder :: Bool -> Bool -> Bool -> (Bool, Bool)
fulladder a b ci = ( (a && b) || ( (xor a b) && ci ), xor ci (xor a b) )

-- Second Element of (Bool, Bool)
saccess :: (Bool, Bool) -> Bool
saccess (f, a) = a

-- First Element of (Bool, Bool)
faccess :: (Bool, Bool) -> Bool
faccess (a, f) = a

-- 4-Bit Adder that does Nibble + Nibble = Nibble
rippleCarryAdder :: Nibble -> Nibble -> Nibble
rippleCarryAdder (a0, a1, a2, a3) (b0, b1, b2, b3) = (saccess (fulladder a0 b0 (faccess (fulladder a1 b1 (faccess (fulladder a2 b2 (faccess (fulladder a3 b3 False))))))), saccess (fulladder a1 b1 (faccess (fulladder a2 b2 (faccess (fulladder a3 b3 False))))), saccess (fulladder a2 b2 (faccess (fulladder a3 b3 False ))), saccess (fulladder a3 b3 False) )

I have the constraint that I can't change the types in the definitions of the functions fulladder and rippleCarryAdder. So one has to work with Nibble.
I would like to know if there is a more elegant way to work with the tuples returned by fulladder without defining a lot of other functions.

rippleCarryAdder is very hard to read. We can make it a lot easier to read if we use local bindings with let … in …:

rippleCarryAdder :: Nibble -> Nibble -> Nibble
rippleCarryAdder (a0, a1, a2, a3) (b0, b1, b2, b3) =
   let (r3, c3) = fulladder a3 b3 False
       (r2, c2) = fulladder a2 b2 c3
       (r1, c1) = fulladder a1 b1 c2
       (r0, _ ) = fulladder a0 b0 c1
   in (r0, r1, r2, r3)

We also don't have to recalculate fulladder a3 b3 False four times.

The functions faccess and saccess already exist and are called fst and snd.


Servlet to let a user download a file Is the code below an efficient way to get the zip file from the server? The code works fine but I'm wondering if I should throw some exceptions for the following conditions:

if(param1 == null || param2 == null)
} else if ("".equals(param1) || "".equals(param2)) {

The way I call this servlet from a web browser is as follows:

https://myserver.com/DownloadFileFromServer/DownloadFileServlet?filename=my_users_files_1560866090.zip&user=TAN

Eventually, I'll call the above URL from my user interface when a user would have clicked a Download button, maybe using Javascript's setTimeout() method. The reason I plan on calling it again and again for sometime is because the file might not be
available on the server and I would have to keep on checkiing it again and again. Do I need to handle this part in my servlet code somehow? I mean, I would keep on callling the above URL until I get the file
from the server.

@WebServlet("/DownloadFileServlet")
public class DownloadFileServlet extends HttpServlet {

    final String[][] contentTypes = {{"zip" ,"application/zip"},{"csv" ,"text/csv"},{"pdf","application/pdf"}};
    private static final long serialVersionUID = 1L;

    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {

        String param1 = request.getParameter("filename");
        String param2 = request.getParameter("user");

        System.out.println("File Name Retrieved is:"+param1);
        System.out.println("User Name Retrieved is:"+param2);

        final String FILE_LOCATION = "/srv/my_users/param2/";

        if(param1 == null || param2 == null) {
            // The Request Parameters Were Not Present In The Query String. Do Something Or Exception Handling !!
            System.out.println("Request Parameter Not Found in first if!");
        } else if ("".equals(param1) || "".equals(param2)) {
            // The Request Parameters Were Present In The Query String But Has No Value. Do Something Or  Exception Handling !!
            System.out.println("Request Parameter Not Found in second if!");
        } else {

        /*if (param1 != null && param2 !=null) {*/
            String fileName = (String) param1;
            String contentType = getContentType(fileName.split("\\.")[1]);
            File file = new File(FILE_LOCATION + "/" +fileName);
            response.setContentType(contentType);
            response.addHeader("Content-Disposition","attachment; filename=" + fileName);
            response.setContentLength((int) file.length());
            ServletOutputStream servletOutputStream = response.getOutputStream();
            BufferedInputStream bufferedInputStream = new BufferedInputStream(new FileInputStream(file));
            int bytesRead = bufferedInputStream.read();
            while (bytesRead != -1) {
                servletOutputStream.write(bytesRead);
                bytesRead = bufferedInputStream.read();
            }
            if (servletOutputStream != null) servletOutputStream.close();
            if(bufferedInputStream != null) bufferedInputStream.close();

        }

        //response.getWriter().append("Served at: ").append(request.getContextPath());
    }

    private String getContentType(String fileType) {
        String returnType = null;
        for(int i = 0; i < contentTypes.length; i++) {
            if(fileType.equals(contentTypes[i][0])) returnType = contentTypes[i][1];
        }
        return returnType;
    }

}

If the parameters are required then they need to be checked and their invalidity must be reported to the caller. There is a HTTP status code for it: 400/BAD REQUEST.

The file transfer is done one byte at a time. This is very inefficient. This kind of transer is usually done in 4096 byte blocks. There are libraries for this task too. For example Apache Commons IO IOUtil.copy(InputStream, OutputStream).

The content type mappings are inefficient. A Map would be a natural fit for the use case instead of a 2D array. But this is something you don't need to do yourself: https://stackoverflow.com/questions/51438/getting-a-files-mime-type-in-java

Calling the servlet repeatedly is called polling. A polling service should be quick and lightweight to not cause excessive load. The client should implement waiting between the polling. The servlet doesn't really need to know about the polling interval. If the servlet should implement throttling of "too eager clients", then you need to implement authentication to identify clients. Depending on the infrastructure you have, you might want to use a firewall or an API gateway for throttling.

By creating the file path by concatenating input from a URL parameter to a path constant you allow the caller to add directories to the path name. E.g. a file name "../../some/sensitive/file.txt" might reveal data you want to keep hidden. It is safer to get a file list from the directory with File.list(FileNameFilter) and use the filter find the file that matches the requested file. Other approach is to sanitize the input, but in mny opinion this is a much more robust solution. If there are a lot of files in the directory and there are a lot of requiests, you might want to cache the file list for a few seconds.

Closing the servlet output stream may have unwanted side effects, so you should just leave it to the servlet container.


Callback C++ Class method from C I am trying to callback C++ class methods from C. Now I have got something working, but am not sure if this is the best solution.
I am looking for feedback, as to if there is something wrong with this approach, and how could I make this better.
So, here goes my code:
This is the C++ class Foo. The class has the methods getValue() & setValue(), which I would like to like to be called-back from the C code:
Contents of Foo.h:
#ifdef __cplusplus
class Foo
{
public:
    Foo();
    int getValue() const;
    void setValue(int value);

private:
    int value_;
};
#endif

// Free functions
int  c_wrapper_getValue(void *arg1);
void c_wrapper_setValue(int value, void *arg1);

Contents of the source file, Foo.cpp:
#include "Foo.h"

Foo::Foo() :
    value_{0}
{}

int Foo::getValue() const
{
    return value_;
}

void Foo::setValue(int value)
{
    value_ = value;
}

void c_wrapper_setValue(int value, void *arg1)
{
    Foo* foo_instance = static_cast<Foo*>(arg1);
    foo_instance->setValue(value);
}

int c_wrapper_getValue(void *arg1)
{
    Foo* foo_instance = static_cast<Foo*>(arg1);
    return foo_instance->getValue();
}

Here is the code for the C files, which in turn call the Foo class members. First, the header Test.h file:
#include <stdio.h>
#include "Foo.h"

#ifdef __cplusplus
extern "C" {
#endif

// Callback function pointers
typedef int  (*get_handler_t)(void* arg1);
typedef void (*set_handler_t)(int foo, void* arg1);

void register_handler(get_handler_t g, set_handler_t s, void* p_instance);

// These are the functions which call the callback functions
int call_cpp_get_function();
void call_cpp_set_function(int value);

#ifdef __cplusplus
}
#endif

And finally the Test.c file:
#include "Test.h"

static get_handler_t get_handler_ = NULL;   /**< Getter function pointer of type int (*g)(void *). */
static set_handler_t set_handler_ = NULL;   /**< Setter function pointer of type void (*s)(int,  void *). */
static void* foo_object_instance  = NULL;    /**< Instance of Foo object. */

//! Registers the getter and setter function with the object of Class Foo
//! \param g Getter function pointer of type int (*g)(void *).
//! \param s Setter function pointer of type void (*s)(int,  void *).
//! \param p_instance Instance of Foo object.
void register_handler(
    int (*g)(void *),
    void (*s)(int,  void *),
    void* p_instance)
{
    get_handler_ = g;
    set_handler_ = s;
    foo_object_instance = p_instance;
}

//! Function calls the get function of class Foo
//! \return Value of value_ from class Foo
int call_cpp_get_function()
{
    return get_handler_(foo_object_instance);
}

//! Function calls the set function of class Foo
//! \param value Value to be set in class Foo
void call_cpp_set_function(int value)
{
    set_handler_(value, foo_object_instance);
}

Now I could create an object and call the functions present in my C file, which called back the C++ methods, like this:
Foo foo_object;
register_handler(&c_wrapper_getValue, &c_wrapper_setValue, static_cast<void *>(&foo_object));
call_cpp_get_function();
call_cpp_set_function(35);

I do have some constraints in my case, for example I can't use STL, since the code is supposed to run on an embedded system. So, how bad is my code?

Encapsulate stuff that shouldn't be public
I'd rather make the callback functions private static functions of the c++ class This helps to encapsulate the nasty void*casts:
#ifdef __cplusplus
class Foo {
public:
    Foo();
    int getValue() const;
    void setValue(int value);

    void register_callbacks();

private:
    int value_;

    static int  cb_getValue(void *arg1);
    static void cb_wrapper_setValue(int value, void *arg1);
};
#endif

Implementation:
void Foo::register_callbacks() {
    register_handler(Foo::cb_getValue, Foo::cb_setValue, static_cast<void *>(this));
}

void Foo::cb_setValue(int value, void *arg1) {
    Foo* foo_instance = static_cast<Foo*>(arg1);
    foo_instance->setValue(value);
}

int Foo::cb_getValue(void *arg1) {
    Foo* foo_instance = static_cast<Foo*>(arg1);
    return foo_instance->getValue();
}

Usage:
Foo foo_object;
foo_object.register_callbacks();
call_cpp_get_function();
call_cpp_set_function(35);


Convert number to X base As an educational exercise, I would like to make this code more efficient, with fewer lines, without using other standard functions.

The goal here is to convert a number to X base and display it, (sample for hexadecimal) including 0x0 => 0, or 0x4096 => 4096.

int     print_hex(int number, int base)
{
    int     i;
    int     len;
    char    c;
    char    buffer[base];

    if (!number)
    {
        putchar('0');
        return 1;
    }
    i = 0;
    while (i < base && number > 0)
    {
        c = "0123456789abcdef"[number % base];
        buffer[i] = c;
        number /= base;
        i++;
    }
    buffer[i] = '\0';
    len = i;
    while (i--)
        putchar(buffer[i]);
    return (len);
}

Your code does not handle negative numbers.  As it is, it will print nothing.  The code should be changed to either handle the negatives properly, or take number as an unsigned where it is not an issue.

Your use of base as the length of the array to store the converted number in is incorrect.  A 32-bit integer will potentially need 32 characters to display the entire number, assuming no zero byte terminator (see below).  buffer should be declared as a fixed size, large enough to hold the longest possible string.  You could then eliminate the i < base check from your while loop.

The initial check of if (!number) can be eliminated by changing your while loop into a do/while look.

If you're looking for brevity in code lines, the body of the loop can be reduced to two statements, although it is more readable with three or four.

The assignment of a 0 byte to the end of buffer is unnecessary in this context since you never pass buffer to a function that requires it.


List-manipulation exercises in Scala I have been using Scala as a means to learn Functional Programming and writing better code. However, I have some questions regarding some exercises or if there's a better way to approach them.

In the exercises below, I have already defined the functions foldLeft, foldRight,append, sum, product, head and tail.

  HARD - Write a function that concatenates a list of lists into a single list. Its runtime should be linear in the total length of all lists. Try to use functions we have already defined.

My solution:

//concatenates list of lists in a single list
def concatLists(x: List[List[Int]]):List[Int] = x.foldLeft( List[Int]() )( (x,y)=> x ++ y )

I have the feeling that the use of foldLeft and the concatenation operator is the right way to go and the runtime feels linear because we are appending one element at a time to the result of the fold, so the time is linear in the input size, am I right? Or is the ++ operator not efficient due to the List being implemented as a linked list? If not, how can I improve this?

  EXERCISE 16: Write a function that transforms a list of integers by adding 1
  to each element. (Reminder: this should be a pure function that returns a new
  List !)

//Adds one - exercise 16
def add1(x:List[Int]):List[Int]= for(elem <- x) yield elem+1

  EXERCISE 18: Write a function map , that generalizes modifying each element in a list while maintaining the structure of the list. Here is its signature: def map[A,B](l: List[A])(f: A => B): List[B]

//implements map - exercise 18
def map[A,B](l: List[A])(f: A => B): List[B]=for(elem <- l) yield f(elem)

I've used yield before in some "toy exercises" in sites like hackerrank and similar and from my understanding, it is used in sequence comprehensions and adds a new element to the resulting sequence, much like in Python and the sequence assumes the return type declared in the function signature, is this correct?

What I want to know is if these implementations are correct in functional terms or if there are other/better ways of writing them.

General Comment

This looks like exercises from Functional Programming in Scala.  It's some time since I read that, but I'm pretty sure you are expected each time to use the functions you have already written, better to learn their use, rather than reach for Scala's core library functions or build-in control structures like for loops.

List Concatenation

Are you using your own implementation or the library version?  Either way, it looks as if you are doing this inefficiently because the first parameter in the library function passed to foldLeft is the accumulator, the second the list element.  As I recall, the book version does the same.

When concatenating immutable lists, the list on the left is entirely recreated, while the list on the right is shared and not copied at all.  foldLeft is the worst solution here because you start with the leftmost list and repeatedly add a new list on the right.  So with the first two lists, you copy the one on the left.  Then, when you add the third on the right, you completely copy your first two lists.  And so on, until you reach the final list.  This means that with n lists, the first list is copied n-1 times, the second n-2 times and so on.  Very wasteful.

With a foldRight solution, where you start by appending the seed (empty list) on the right of the rightmost list and then in each subsequent operation append the accumulated value on the right and not the left, each list is only copied once.

By the time you reached this challenge, you should have written a stack-safe version of foldRight.

You could, alternatively, reverse the outer list and then foldLeft.  Either way, remember that you want the accumulator to be on the right hand side of ++

add1

By using a for loop you have learned nothing about how to use the functions you have already written to build other functions.

You should, at this point, have written two stack-safe functions which make it possible to traverse a list: foldLeft and foldRight.  Either can be used but remember that you need to preserve the original order.

map

Again, using a for loop you have learned nothing about how to use the functions you have already written.  In fact, since Scala will actually rewrite the for loop using the core library map function, you've implemented map using map.
 Which works buy isn't in the spirit of the challenge.

map is just the general case of add1, so the same answer applies as in that challenge.

Summary

Your first solution didn't meet the requirements at all, since it scales exponentially and not linearly.  The other two work functionally but don't meet the explicitly given (and often repeated) requirement to reuse functions you've written.

Forgive me for not providing you with any code examples but I don't want to rob you of the challenge to learn (which is the whole point of the book).


Update first and last item in List I am trying to do the following. I have a list of Custom objects:

public class TestObject {
  public int Id { get; set; }
  public string Name { get; set; }
  public bool First { get; set; }
  public bool Last { get; set; }
}

I want to set the property First and Last to true at the first and last item in the list.

Side note: I am filling the list in a LinqToEntity query. Perhaps its even possible in LinqToEntity.

I have come up with the following but i doubt its the most efficient way.

var result = new List<TestObject> {
   new TestObject { Id = 1, Name = "Test1" },
   new TestObject { Id = 2, Name = "Test2" },
   new TestObject { Id = 2, Name = "Test2" }
};

foreach (var item in result)
{
   item.First = result.FirstOrDefault() == item;
   item.Last = result.LastOrDefault() == item;
}

I eventually want to display the results in a ASP.Net's ListView (webforms), where I have buttons that will be displayed based on the property.

Is there an easier or more efficient way of doing this?

The performance of your current code depends a little about the implementation of the list, but what I primarily would have done is to extract first and last to variables that get initialized once, not in each iteration.

var first = result.FirstOrDefault();
var last = result.LastOrDefault();
foreach (var item in result)
{
   item.First = first == item;
   item.Last = last == item;
}

Secondly, if you're sure that all initial items have false at both the First and Last properties, then you don't need to use a loop at all, just do something like this:

if (result.Any())
{
    result.First().First = true;
    result.Last().Last = true;
}

I am no C# expert but this should give you an idea.

However, I'd question why you have the First and Last properties in the first place. Are you really really sure that you need them as properties? It feels to me that there's a risk that they will be inconsistent with their actual position in the list. And if you have access to the list itself where you want to get the value of the First/Last properties, well then you don't need the properties at all, do you?


Calling a global popup from nested vue components I have tons of nested Vue components that need to be able to open a global modal. To avoid having tons of listeners and emitters snaking their way all through the app, I am using VueX. The main App has a modal which gets shown if the modal state is not undefined.

Root app template

<template>
   <components></components>
   <popup v-if="popupImage" :image="popupImage"></popup>
</template>

Root app script

import { State } from 'vuex-class'
@State(state => state.popupImage) popupImage:string

Nested component

From within a component I set the popup string, which will trigger the global modal:

this.$store.commit("setPopup", "bla.jpg")

It all works fine, but does it seem a bit overkill to add VueX ONLY for a modal window? The rest of the app doesn't use VueX at all. What is the recommended way to open a modal from anywhere in a Vue app?

Global Event Bus is a solution but since you have to listen emits in components's mounted or created lifecycle hook, it's getting duplicated in each instance.

I had same 'issue' and resolved it by mounting my popup component dynamically and setting it into prototype of my Vue instance.

Example;

import Vue from 'vue';
import Popup from '@/components/Popup.vue';

Vue.prototoype.$popup = new Vue(Popup).$mount();

Imagine you have show method in your Popup component. Now with this injection you can use it in any component as;

this.$popup.show();


Thread that monitors a web API to get the latest announcement via JSON I'm working to make a thread that monitors a web API to get the latest announcement via JSON. I cannot test this currently, so I'm unsure if anything needs to be changed with this. I've read through other questions but everyone else doesn't seem to be using a loop to keep getting a response.

public void run(){
    try {
        URL url = new URL(announcementsURL);
        HttpURLConnection http = (HttpURLConnection) url.openConnection();
        http.setRequestMethod("GET");
        http.setRequestProperty("Connection", "keep-alive");
        http.setUseCaches(false);
        http.setAllowUserInteraction(false);
        http.setConnectTimeout(10);
        http.setReadTimeout(10);
        while (true){
            http.connect();
            int status = http.getResponseCode();
            if (status == 201){
                BufferedReader br = new BufferedReader(new InputStreamReader(http.getInputStream()));
                StringBuilder sb = new StringBuilder();
                String line;
                while ((line = br.readLine()) != null) {
                    sb.append(line+"\n");
                }
                br.close();
                String json = sb.toString();
                JSONParser parser = new JSONParser();
                JSONObject jsonResponse = (JSONObject) parser.parse(json);
                if (!(lastAnnouncement == (long) jsonResponse.get("time"))){
                    //String announcement = (String) jsonResponse.get("message");
                    //TODO What to do with announcement...
                }
            }
            http.getInputStream().close();
            http.disconnect();
        }

    } catch (IOException | ParseException e) {
        e.printStackTrace();
        this.interrupt();
        try {
            this.join();
        } catch (InterruptedException e1) {
            e1.printStackTrace();
        }
    }
}

Immediate observation: 201 CREATED is a really weird status code to expect here.  The GET method is supposed to be both safe and idempotent.  201 is supposed to indicate that a new resource has been created in response to the request.  Those don't go together.

Also, if you are polling for changes, you'll normally want to send a "conditional GET".  The web server should be providing you with an ETag that you can use, and your client should be sending an If-None-Match header with the previously seen ETag, allowing the server to save some work and bandwidth by returning a 304 NOT MODIFIED

This implementation is also blasting the host with new connections.  Check your requirements -- given that you are talking to the server across the network, there is clearly some acceptable latency.  If the SLA really is so tight that you have to poll the host constantly, then you should look into Long Polling; if it isn't, then your loop should be spacing out the requests.

You didn't include anything but your run method, but this code fragment

    e.printStackTrace();
    this.interrupt();
    try {
        this.join();
    } catch (InterruptedException e1) {
        e1.printStackTrace();
    }

Stongly suggests that you've written a Thread.  Don't do that.  Let the standard libraries do the work of managing the threads.  Instead, you should create a Runnable, and submit it to an ExecutorService to do the work.

A ScheduledExecutorService supports scheduledAtFixedDelay.  You could use that to poll the server at a nice friendly interval, rather than writing a tight loop that tries to DOS the host all by its lonesome.

Your run() method is doing too many different things, and should be refactored.  Single responsibility principle suggests that methods should either create, coordinate, or calculate.  This code is doing all three -- you are coordinating the client server protocol, managing an http connection and an http input stream, creating a string buffer and a json parser, reading the response entity, parsing the entity, doing some unspecified work with the result.... Create different methods (possibly in different objects) to share the work.


Recursive method turning flat structure to recursive I often end up working with systems where the data is delivered with some Id property and possibly a parentId prop, but never do anyone tempt to actually give me a nice recursive object to play with.

Now, the code below is merely a suggestion in how to solve these kind of "problems", I've created the FlatObject to simulate an incoming  with data.

class Program
{
    static void Main(string[] args)
    {
        // Fill list with sample data
        List<FlatObject> flatObjects = new List<FlatObject>
        {
            new FlatObject(1,0),
            new FlatObject(2,0),
            new FlatObject(3,0),
            new FlatObject(4,0),
            new FlatObject(5,1),
            new FlatObject(6,2),
            new FlatObject(7,6),
            new FlatObject(8,6)
        };

        // call the recursive method
        var recursiveObjects = FillRecursive(flatObjects, 0);

    }

    private static List<RecursiveObject> FillRecursive(List<FlatObject> flatObjects, int parentId)
    {
        List<RecursiveObject> recursiveObjects = new List<RecursiveObject>();

        foreach (var item in flatObjects.Where(x => x.ParentId.Equals(parentId)))
        {
            recursiveObjects.Add(new RecursiveObject
            {
                Id = item.Id,
                ParentId = item.ParentId,
                Children = FillRecursive(flatObjects, item.Id)
            });
        }

        return recursiveObjects;
    }
}

public class FlatObject
{
    public int Id { get; set; }
    public int ParentId { get; set; }

    public FlatObject(int id, int parentId)
    {
        Id = id;
        ParentId = parentId;
    }
}

public class RecursiveObject
{
    public int Id { get; set; }
    public int ParentId { get; set; }
    public List<RecursiveObject> Children { get; set; }
}

I am aware of some Linq alternatives to solve the foreach loop but that does hardly not change the approach of this.

private static List<RecursiveObject> FillRecursive(List<FlatObject> flatObjects, int parentId)
{
    return flatObjects.Where(x => x.ParentId.Equals(parentId)).Select(item => new RecursiveObject
    {
        Id = item.Id, ParentId = item.ParentId, Children = FillRecursive(flatObjects, item.Id)
    }).ToList();
}

Below is the code for my approach.  A benefit is that the heirarchy goes both ways; up and down.  While the parent object contains a list of child objects, each child has a reference to the parent object.

Some differences from your original setup:

The ParentId property on FlatObject is nullable, so objects that have no parent will have null.
The Children property on RecursiveObject is IEnumerable instead of List.  This prevents consumers from modifying the contents of the list.  If you want to allow consumers to add/remove items from the list, then RecursiveObject should expose methods to perform those actions so that you can make sure that the Parent property is properly assigned when a child is added/removed.
I've made the Id, Parent, and Children properties on RecursiveObject read-only.  This certainly isn't necessary, but you can do that if desired.  This is the reason why I made the FlatObjectsToRecursiveObjects method as a static method of the RecursiveObjects class; so that it can have access to the private property setters.

The gist of this approach is that we first convert the FlatObjects to RecursiveObjects and store them in a dictionary keyed by the Id.  Then, we do another pass over the FlatObjects in order to assign the Parent and Children properties of each RecursiveObject, using the dictionary to perform the necessary look-ups with the FlatObject's Id and ParentId properties.

class FlatObject
{
    public int Id { get; set; }
    public int? ParentId { get; set; }
}

class RecursiveObject
{
    public int Id { get; private set; }
    public RecursiveObject Parent { get; private set; }
    public IEnumerable<RecursiveObject> Children { get; private set; }

    public static IEnumerable<RecursiveObject> FlatObjectsToRecursiveObjects(IEnumerable<FlatObject> flatObjects)
    {
        // convert flat objects into heirarchial objects and store them in a dictionary keyed with the object's Id
        var recursiveObjectsById = flatObjects.Select(item => new RecursiveObject { Id = item.Id }).ToDictionary(item => item.Id);

        // group flat objects by their ParentId
        var flatObjectsGroupedByParentId = flatObjects.Where(item => item.ParentId.HasValue).GroupBy(item => item.ParentId.Value);
        foreach (var group in flatObjectsGroupedByParentId)
        {
            // find each group's parent object
            RecursiveObject parent;
            if (recursiveObjectsById.TryGetValue(group.Key, out parent))
            {
                // convert the group of flat objects to a list of heirarchial objects by looking up the correct heirarchial object from the dictionary
                parent.Children = group.Select(item => recursiveObjectsById[item.Id]).ToList();

                // assign the parent object to each child object
                foreach (var child in parent.Children)
                {
                    child.Parent = parent;
                }
            }
            else
            {
                // something's wrong!!!  ParentId refers to a non-existant object.
            }
        }

        return recursiveObjectsById.Values;
    }
}

EDIT:  Added an "improved" version of my original approach, shown below.  The following implementation provides the following benefits:

The Children property on RecursiveObject never returns a null, but it still retains the "benefit" of lazy initialization.  The Children property getter checks if the _Children field is null, and if so it returns the default instance of the EmptyEnumerable<RecursiveObject> class (code included).
Children can now be added/removed using the new AddChild, RemoveChild, and AddChildren methods on RecursiveObject.
The FlatObjectsToRecursiveObjects method is slightly simpler now because it utilizes the new AddChildren method.
The FlatObjectsToRecursiveObjects method no longer has to be a member of the RecursiveObject class, since it does not access any private details of the class.
My setup code includes a second root (new FlatObject(9,-1)) and circular references (new FlatObject(10,10) and new FlatObject(0,2)), just to verify that the implementation can handle these special cases.

class FlatObject
{
    public int Id { get; set; }
    public int? ParentId { get; set; }

    public FlatObject(int id)
    {
        this.Id = id;
    }

    public FlatObject(int id, int parentId)
        : this(id)
    {
        this.ParentId = parentId;
    }
}

class RecursiveObject
{
    public int Id { get; private set; }
    public RecursiveObject Parent { get; private set; }
    private List<RecursiveObject> _Children;

    public IEnumerable<RecursiveObject> Children
    {
        get
        {
            IEnumerable<RecursiveObject> value = _Children;
            if (value == null)
                value = EmptyEnumerable<RecursiveObject>.Default;
            return value;
        }
    }

    public RecursiveObject(int id)
    {
        this.Id = id;
    }

    public void AddChild(RecursiveObject child)
    {
        if (_Children == null)
            _Children = new List<RecursiveObject>();
        _Children.Add(child);
        child.Parent = this;
    }

    public bool RemoveChild(RecursiveObject child)
    {
        if (_Children != null)
        {
            bool removed = _Children.Remove(child);
            if (removed)
                child.Parent = null;
            return removed;
        }
        else
        {
            return false;
        }
    }

    public void AddChildren(IEnumerable<RecursiveObject> children)
    {
        if (children == null)
            throw new ArgumentNullException("children");

        if (_Children == null)
            _Children = new List<RecursiveObject>(children);
        else
            _Children.AddRange(children);

        foreach (var child in children)
        {
            child.Parent = this;
        }
    }
}

class Program
{
    public static IEnumerable<RecursiveObject> FlatObjectsToRecursiveObjects(IEnumerable<FlatObject> flatObjects)
    {
        // convert flat objects into heirarchial objects and store them in a dictionary keyed with the object's Id
        var recursiveObjectsById = flatObjects.Select(item => new RecursiveObject(item.Id)).ToDictionary(item => item.Id);

        // group flat objects by their ParentId
        var flatObjectsGroupedByParentId = flatObjects.Where(item => item.ParentId.HasValue).GroupBy(item => item.ParentId.Value);
        foreach (var group in flatObjectsGroupedByParentId)
        {
            // find each group's parent object
            RecursiveObject parent;
            if (recursiveObjectsById.TryGetValue(group.Key, out parent))
            {
                // convert the group of flat objects to a list of heirarchial objects by looking up the correct heirarchial object from the dictionary
                parent.AddChildren(group.Select(item => recursiveObjectsById[item.Id]));
            }
            else
            {
                // something's wrong!!!  ParentId refers to a non-existant object.
            }
        }

        return recursiveObjectsById.Values;
    }

    static void Main(string[] args)
    {
        List<FlatObject> flatObjects = new List<FlatObject>()
            {
                new FlatObject(1,0),
                new FlatObject(2,0),
                new FlatObject(3,0),
                new FlatObject(4,0),
                new FlatObject(5,1),
                new FlatObject(6,2),
                new FlatObject(7,6),
                new FlatObject(8,6),
                new FlatObject(9,-1),
                new FlatObject(10,10),
                new FlatObject(0,2),
            };

        var recursiveObjects = FlatObjectsToRecursiveObjects(flatObjects).ToList();
    }
}

#region Universal Code

class EmptyEnumerator<T> : IEnumerator<T>
{
    public static readonly EmptyEnumerator<T> Default = new EmptyEnumerator<T>();

    public T Current
    {
        get { throw new InvalidOperationException(); }
    }

    public void Dispose()
    {
    }

    object System.Collections.IEnumerator.Current
    {
        get { throw new InvalidOperationException(); }
    }

    public bool MoveNext()
    {
        return false;
    }

    public void Reset()
    {
    }
}

class EmptyEnumerable<T> : IEnumerable<T>
{
    public static readonly EmptyEnumerable<T> Default = new EmptyEnumerable<T>();

    public IEnumerator<T> GetEnumerator()
    {
        return EmptyEnumerator<T>.Default;
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        return this.GetEnumerator();
    }
}

#endregion


Interval Subtraction From the Extended Exercise beginning in section 2.1.4, you can find exercise 2.8:

  Exercise 2.8.  Using reasoning
  analogous to Alyssa's, describe how
  the difference of two intervals may be
  computed. Define a corresponding
  subtraction procedure, called
  sub-interval.

I wrote the following:

(define (sub-interval x y)
  (make-interval (- (lower-bound x) (upper-bound y))
                 (- (upper-bound x) (lower-bound y))))

What do you think?

Looks great.  Don't forget to define lower-bound and upper-bound as car and cdr respectively.


Writing Merge Sort in C++ I am learning programming online and currently watching CS50 lectures, in third lecture, professor introduces us to merge_sort though only through pseudo code, so I wanted someone to review my implementation and tell me where I can improve and what I possibly missed.

#include <algorithm>
#include <assert.h>
#include <cmath>
#include <iostream>
#include <vector>

template <typename T> void merge_sort(T m_array[], const size_t size) {
    using namespace std;
    if (size < 2)
        return;
    else if (size == 2) {
        if (m_array[0] > m_array[1])
            std::swap(m_array[0], m_array[1]); // no need for a new array
        return;
    }
    merge_sort(m_array, size / 2);
    merge_sort(m_array + size / 2,
               std::ceil(static_cast<double>(size) /
                         2) // for odd size, divison will have truncated value
                            // and recursion will left the the last array value
                            // for sorting, size%2==0?size/2:size/2+1
    );

    T *new_array = new T[size];
    size_t na_pos = 0;

    const T *array1 = m_array, *array2 = m_array + size / 2;

    const T *const array1end = m_array + size / 2, *const array2end =
                                                       m_array + size;

    while (1) {
        if (array1 == array1end)
            break;
        if (array2 == array2end)
            break;

        if (*array1 < *array2)
            new_array[na_pos++] = *array1++;
        else if (*array2 < *array1)
            new_array[na_pos++] = *array2++;
        else if (*array1 == *array2) {
            new_array[na_pos++] = *array1++;
            new_array[na_pos++] = *array2++;
        }
    }

    assert(array1 == array1end || array2 == array2end);

    while (array1 != array1end)
        new_array[na_pos++] = *array1++;
    while (array2 != array2end)
        new_array[na_pos++] = *array2++;
    assert(na_pos == size);

    for (na_pos = 0; na_pos < size; na_pos++)
        m_array[na_pos] = new_array[na_pos];

    delete[] new_array;
}

int main() {
    using namespace std;

    auto limit = 10000;
    const auto size = 21;
    while (limit--) {
        vector<int> a;
        a.reserve(size);
        while (a.size() < size)
            a.push_back(rand() % 10000);
        merge_sort(a.data(), a.size());
        assert(std::is_sorted(a.begin(), a.end()));
    }

    return 0;
}

std::ceil(static_cast<double>(size) / 2 is a very long (and unclear) way to say size - size/2.
No naked loops please. Each loop represents an important algorithm, and deserves a name. For example,

while (array1 != array1end)
    new_array[na_pos++] = *array1++;

wants to be copy(new_array + na_pos, array1, array1_end);, and you may reuse it at least 2 more times (and don't make it void: it has something interesting to return).

Similarly, the merging loop represents a merge algorithm.
The block

if (*array1 < *array2)
    new_array[na_pos++] = *array1++;
else if (*array2 < *array1)
    new_array[na_pos++] = *array2++;
else if (*array1 == *array2) {
    new_array[na_pos++] = *array1++;
    new_array[na_pos++] = *array2++;
}

introduces a subtle problem. Here your algorithm may lose stability. If array1 has two elements comparing equal, say x1,x2, and array2 has an element x3 which also compares equal to them, the final arrangement will be x1, x3, x2, whereas the stable sort should result in x1, x2, x3.

The problem is easy to fix:

if (*array1 <= *array2) {
    new_array[na_pos++] = *array1++;
} else {
    new_array[na_pos++] = *array2++;
}

I strongly recommend to put the loop termination conditions where they belong:

while ((array1 != array1end) && (array2 != array2end))


Advent of Code 2016: Day 1 "No Time for a Taxicab" Here I'm talking about Part 2 of the puzzle, where we need to find "How many blocks away is the first location you visit twice?".

At first I solved it as everybody else using a set of all visited points.
Peter Norvig employed that approach, you can check his solution on GitHub.

My code was not that elegant, but Peter's notebook reminded me of existence of complex type, so I modified my function and ended up here:

def solve_set(instructions):
    direction, loc, visited = 1j, 0, {0}
    for i in instructions.split(', '):
        direction *= {'R': -1j, 'L': 1j}[i[0]]
        for _ in range(int(i[1:])):
            loc += direction
            if loc in visited:
                return abs(loc.real) + abs(loc.imag)
            visited.add(loc)

Now let's try to optimize it.

The puzzle uses Taxicab geometry,
and if we assume that all instructions have a positive number of steps (greater than zero),
we can split segments into two subsets, parallel and orthogonal,
and for each new segment check only orthogonal segments for an intersection.
No matter what, on a new instruction (i.e. new segment) those two sets interchange,
so parallel segments become orthogonal and orthogonal become parallel.
To update subsets (represented as lists) we use insort function from bisect module
which inserts an element into a list keeping it sorted.
Then we get a list of candidates from orthogonal set of segments using bisect function
and iterate over it in the direction of current segment to get the first intersection:

from bisect import bisect, insort

def solve_bisect(instructions):
    N, E, S = 1j, 1, -1j
    direction, cur, prev, prev_s = N, 0, 0, None
    parallel, orthogonal = [], []  # segments
    for i in instructions.split(', '):
        direction *= {'R': -1j, 'L': 1j}[i[0]]
        cur += direction * int(i[1:])
        if direction in (N, S): s = prev.real, prev.imag, cur.imag
        else:                   s = prev.imag, prev.real, cur.real
        insort(parallel, s)
        candidates = orthogonal[bisect(orthogonal, s[1:2]) : bisect(orthogonal, s[2:])]
        for c in candidates if direction in (N, E) else candidates[::-1]:
            if prev_s != c and (c[1] <= s[0] <= c[2] or c[2] <= s[0] <= c[1]):
                return abs(s[0]) + abs(c[0])
        prev, prev_s, parallel, orthogonal = cur, s, orthogonal, parallel

Now we need to generate a large input for comparison:

from random import choice, randrange
instructions = ', '.join(f'{choice("LR")}{randrange(1, 1000000)}' for _ in range(100000))

I don't know what you will end up with, but I get these results:

In [15]: %timeit solve_bisect(instructions)
5.91 ms ± 144 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [16]: %timeit solve_set(instructions)
465 ms ± 8.75 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

So in this case solve_bisect is roughly 100x times faster than the naive approach.

But I'd like to know if there's an even better way to solve the problem.

Before starting to make timing comparisons, it's important to check that the optimized code is correct. It's no good being a hundred times faster if you get the wrong answer! So let's look at the results of the two functions on a simple test case:

>>> instructions = 'L1, L1, L1, L1, R1, R1, R1, R1'
>>> solve_set(instructions)
0.0
>>> solve_bisect(instructions)
1.0

Oops.


Backing up an entire website with PHP I'm attempting to backup lots of website using this php code. The script basically goes through the whole public_html directory and adds all the folders and files recursively and zips them.

Here's what I've got so far:

if (!file_exists('/home/sites/'.$_SERVER['SERVER_NAME'].'/cbitsbackup/')) {
mkdir('/home/sites/'.$_SERVER['SERVER_NAME'].'/cbitsbackup/', 0777, true);
}
$filefront = '/home/sites/'.$_SERVER['SERVER_NAME'].'/cbitsbackup/backup-'.$_SERVER['SERVER_NAME'].'-day1';
$fileend = '.zip';
$time = time();

if(file_exists($filefront.$fileend)){
$file = $filefront.'-'.$time.$fileend;}
else{$file = $filefront.$fileend;};
zip_directory('/home/sites/'.$_SERVER['SERVER_NAME'].'/public_html',$file);

function zip_directory($source,$tempfile){
if(!extension_loaded('zip') || !file_exists($source)) return false;
$zip = new ZipArchive();
if(!$zip->open($tempfile,ZIPARCHIVE::CREATE)) return false;
$source = str_replace('\\','/',realpath($source));
if(is_dir($source) === true){
    $files = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($source), RecursiveIteratorIterator::SELF_FIRST);
    foreach($files as $file){
        $file = str_replace('\\', '/', realpath($file));
        if(is_dir($file) === true) $zip->addEmptyDir(str_replace($source . '/','', $file . '/'));
        else if(is_file($file) === true) $zip->addFromString(str_replace($source . '/', '', $file), file_get_contents($file));}}
elseif(is_file($source) === true) $zip->addFromString(basename($source), file_get_contents($source));
return $zip->close();}

The main problem at the moment is memory constraints (64M) because I'm running on a shared hosting platform. As far as I can see, the zip file is stored in memory until it is 'completed'. This is problem because I have a lot of websites which are much over this size. Is there anyway to write the zip file progressively to disk so my script isn't killed by the system? Could I even write this in Perl to get around this limit?

As you are using Linux, use this in a PHP file:

<?php system('zip -r backup *'); ?>

This will create a ZIP of all files/directories exists in the directory.


Refactoring Java transmitter delegate implementation I have 3 classes: a transmitter, a view for displaying and a recording view for flushing the screen contents into a file. The view and the recording view classes register themselves from their constructor in the transmitter, which keeps them in a HashMap. The transmitter implements all the view and recording view classes' interfaces, and delegates the incoming method invocations to them based on the actually registered implementors.

To make things clear:

public interface IRecordingView {
  public void recordingViewMethod1();
  public void recordingViewMethod2();
}

public class RecordingView implements IRecordingView {
  public RecordingView {
    Transmitter.getInstance().register(this);
  }

  @Override public void recordingViewMethod1() { ... }
  @Override public void recordingViewMethod2() { ... }
}

public interface IView {
  public void viewMethod1();
  public void viewMethod2();
}

public class View implements IView {
  public View() {
    Transmitter.getInstance().register(this);
  }

  @Override public void viewMethod1() { ... }
  @Override public void viewMethod2() { ... }
}

public class Transmitter implements IRecordingView, IView {
  private static final Transmitter instance = new Transmitter();
  private HashMap<Class, Set> register = new HashMap<Class, Set>();

  public Transmitter() {
    for (Class cl : this.getClass().getInterfaces()) {
      register.put(cl, new HashSet());
    }
  }

  public void register(Object listener) {
    for (Class interf : register.keySet()) {
      if (interf.isInstance(listener)) {
        Set implementors = register.get(interf);
        implementors.add(listener);
      }
    }
  }

  @Override public void viewMethod1() {
    for (IView delegate : register.get(IView.class)) {
      delegate.viewMethod1();
    }
  }

  @Override public void recordingViewMethod1() {
    for (IRecordingView delegate : register.get(IRecordingView.class)) {
      delegate.recordingViewMethod1();
    }
  }

  // and so on for the rest of the methods
}

This is the basic concept of the legacy code. Of course there are many transmitters, each implementing more interfaces, with each interface having more methods. Is there an optimal and/or elegant way to further reduce code size? The delegation mechanism looks quite the same in every method, but I don't want to mess with reflection.

Why no reflection? Have you considered code generation (compile time or runtime)?

Here's an idea based on proxies and reflection. I haven't actually tried any of this, so the idea might break down at some point, but here goes.

Create an interface for the transmitter that extends all of the interfaces the transmitter will implement:

public interface ITransmitter extends IRecordingView, IView { ... }

Have the transmitter class implement that interface and return the singleton instance as that interface:

public class Transmitter implements ITransmitter {
    public static ITransmitter getInstance() { ... }
}

The instance returned is actually a proxy created using java.lang.reflect.Proxy with an InvocationHandler that will delegate the appropriate method invocations using reflection.


ElasticSearch, Tire - refactor autocomplete method for multiple resources I am using ElasticSearch and Tire for search in my app across all models.

I would like to refactor autocomplete method because it looks to complex for me.
Am I using a good pattern for searching across model?

Model

class Link < ActiveRecord::Base
  include Tire::Model::Search
  include Tire::Model::Callbacks

  tire.mapping do
    indexes :id, :type => 'string', :index => :not_analyzed
    indexes :title, analyzer: 'snowball', boost: 100
  end

  def to_indexed_json
    to_json(
      only: [:id, :title]
    )
  end
  ...
end

Controller

class SearchController < ApplicationController
  def new
    search = SearchesCatalog.new(params[:term])

    render json: search.autocomplete.to_json
  end
end

Lib

class SearchesCatalog
  attr_reader :options

  def initialize(options = {})
    @options = HashWithIndifferentAccess.new(options)
    @resources = [ :questions, :answers, :links, :events, past_events, :reviews ]
  end

  def results
    term = options[:term]

    Tire.search @resources do
      query { string "#{term}" }
    end.results
  end

  def autocomplete
    array = []
    results.each do |result|
      case result.type
    when "past_event"
      array << { title: result.content, label: "#{result.content} PastEvent", value: "/past_events/#{result.id}" }
    when "event"
      array << { title: result.title, label: "#{result.title} Event", value: "/events/#{result.id}" }
    when "topic"
      array << { title: result.name, label: "#{result.name} Topic", value: "/#{result.app_id}/t/#{result.id}" }
    when "link"
      array << { title: result.title, label: "#{result.title} Link", value: "/links/#{result.id}" }
    when "question"
      array << { title: result.content, label: "#{result.content[0..100]} Question" }
    when "answer"
    array << { title: result.content, label: "#{result.content[0..100]} Answer" }
    end
    array
  end
end

Javascript

// set up the search bar
$("#question_search").autocomplete({
  source:$('#question_search').data('source'),
  html: true,
  minlength: 1,
  appendto: "#search_results",
  select: function( event, ui ) {
    window.location=ui.item.value;
    return false;
  },
  focus: function( event, ui ) {
    $("#question_search").val(ui.item.title);
    return false;
  },
  open: function( event, ui ) {
    $('#search_results')
    .find('a:first').addclass('first').end()
    .find('a:last').addclass('last');
  }
});

I'll take autocomplete and leave the rest for others. Some notes:

Never write the pattern "empty array" + each + push + return array. That's a map (more on functional programming here)
Is this "lib" thing really in lib/? it should go to app/. In lib you only have generic code that may be reused across applications.
Don't write routes by hand, ever, that's terrible practice, use the helper router methods provided by Rails.
Don't write tags by hand, use content_tag.
Abstract and simplify by identifying repeated patterns in your code.

I'd write:

def autocomplete
  results.map do |result|
    title, caption, route = case result.type.to_sym
    when :past_event
      [result.content, "PastEvent", past_event_path(result)]
    ...
    end

    label = "%s %s" % [title, content_tag(:span, caption, class: 'search-type')]
    {title: title, label: label, value: route}
  end
end


Set + Queue + a get method backed by a Map The other day I stumbled upon this question on Stack Overflow about the lack of a get method on the Set and loved the suggestion of using a Map<E,E> to back the Set. I decided to try this out. Since this is quite a generic object that could serve a lot of purposes, I would like to know if there is something weird with my implementation.

NB: I am using Java 7.

Here is the interface:

public interface QueuedSetWithGet<E> extends Set<E>, Queue<E> {
    public E get(E e);
}

And here is the implementation I wrote:

public class QueuedSetWithGetImpl<E> implements QueuedSetWithGet<E> {

    LinkedHashMap<E, E> map = new LinkedHashMap<E, E>();

    @Override
    public int size() {
        return map.size();
    }

    @Override
    public boolean isEmpty() {
        return map.isEmpty();
    }

    @Override
    public boolean contains(Object paramObject) {
        return map.containsKey(paramObject);
    }

    @Override
    public Iterator<E> iterator() {
        return map.keySet().iterator();
    }

    @Override
    public Object[] toArray() {
        return map.keySet().toArray();
    }

    @Override
    public <T> T[] toArray(T[] paramArrayOfT) {
        return map.keySet().toArray(paramArrayOfT);
    }

    @Override
    public boolean add(E paramE) {
        boolean notAlreadyPresent = !map.containsKey(paramE);
        if (notAlreadyPresent) {
            map.put(paramE, paramE);
        }
        return notAlreadyPresent;
    }

    @Override
    public boolean remove(Object paramObject) {
        return map.remove(paramObject) != null;
    }

    @Override
    public boolean containsAll(Collection<?> paramCollection) {
        return map.keySet().containsAll(paramCollection);
    }

    @Override
    public boolean addAll(Collection<? extends E> paramCollection) {
        boolean mapChanged = false;
        for (E e : paramCollection) {
            mapChanged |= this.add(e);
        }
        return mapChanged;
    }

    @Override
    public boolean retainAll(Collection<?> paramCollection) {
        boolean mapChanged = false;
        for (Object e : paramCollection) {
            if (!this.contains(e)) {
                mapChanged = true;
                this.remove(e);
            }
        }
        return mapChanged;
    }

    @Override
    public boolean removeAll(Collection<?> paramCollection) {
        boolean mapChanged = false;
        for (Object e : paramCollection) {
            mapChanged |= this.remove(e);
        }
        return mapChanged;
    }

    @Override
    public void clear() {
        map.clear();
    }

    @Override
    public boolean offer(E paramE) {
        return this.add(paramE);
    }

    @Override
    public E remove() {
        E head = this.element();
        this.remove(head);
        return head;
    }

    @Override
    public E poll() {
        return this.isEmpty() ? null : this.remove();
    }

    @Override
    public E element() {
        E e = this.peek();
        if (e == null) {
            throw new NoSuchElementException("queue is empty");
        } else {
            return e;
        }
    }

    @Override
    public E peek() {
        return this.isEmpty() ? null : map.keySet().iterator().next();
    }

    @Override
    public E get(E e) {
        return map.get(e);
    }
}

Bug in retainAll

The method retainAll does not respect its contract. It is supposed to remove from this set all the elements that are not contained in the given set. However, the following code:

Set<String> set = new QueuedSetWithGetImpl<>();
set.add("foo");
set.add("bar");
set.retainAll(Arrays.asList("foo"));
System.out.println(set.size()); // prints 2!

does not remove the "bar" element from the set (the size is still 2).

The issue with the current code is that it doesn't loop over the elements of the set to find the ones not contained in the given collection. To implement it correctly, you can use an Iterator and call remove():

@Override
public boolean retainAll(Collection<?> paramCollection) {
    boolean mapChanged = false;
    Iterator<E> it = iterator();
    while (it.hasNext()) {
        if (!paramCollection.contains(it.next())) {
            mapChanged = true;
            it.remove();
        }
    }
    return mapChanged;
}

Access control

LinkedHashMap<E, E> map = new LinkedHashMap<E, E>();

This map needs to be private. It is the backing map of your implementation and is not intended to be used by other classes.

Possible simplification

boolean notAlreadyPresent = !map.containsKey(paramE);
if (notAlreadyPresent) {
    map.put(paramE, paramE);
}
return notAlreadyPresent;

This can be simplified a bit by using the result of put, which is either the previous value or null if there was no mapping. As such, you can have:

return map.put(paramE, paramE) == null;

Note that will replace the old value with the given new value, but it won't break the contract of the Set which defines equality in terms of equals and not ==. To not replace, you could use putIfAbsent on Java 8, but since you're using Java 7, you could keep what you have.