Rewrite new function to object

[adwydman]

Here is my proposition: rewriting new function to object literals/assignments.

Why?

Basically Sealious uses new function on two different occasions:

1. When declaring an object.
2. When declaring a prototype.

The main goal of that approach was to hide the private fields and expose only the public ones like this:

const Test = new function() {
    function lol() { }
    this.printLol = function() { 
        lol();
    }
}

module.exports = Test;

This is unnecessary, as JavaScript supports closerues, therefore functions remember the environment they were created in. The above example work the same as this one:

function lol() {}

const Test = {
    printLol: function() {
        lol();
    }
}

module.exports = Test;

Prototypes

The same applies to prototypes, plus one more thing:

AccessStrategy.prototype = new function(){

    this.check = function(context, item){
        return Promise.resolve(this.type.check(context, this.params, item));
    }

    this.is_item_sensitive = function(){
        return this.type.is_item_sensitive(this.params);
    }
}

This prototype is overwritten by the new function provided and I wonder if this is the best idea.

Summary

We spoke about this before and maybe the time has come to rewrite new function. :) Let me know what you think. If the response is positive, I will carry on.

[kuba-orlik]

Great Idea!

1. It's hard to assess the corectness of the changes when we don't have working tests, so please hold on.
2. Once the tests are written and all of the new functions are replaced in this branch, it'll be a good merge candidate :)

This is unnecessary, as JavaScript supports closerues, therefore functions remember the environment they were created in.

And, in order to ease unit testing, we should minimize usage of private properties overall (not that we have that many, anyway)

[adwydman]

I have rewritten every file I found. The exception is core.js, because it will probably be deleted.

[adwydman]

@kuba-orlik It's ready to be reviewed.

[kuba-orlik]

Hmmm I've tried running the tests locally and they failed:

> sealious@0.7.11 test /home/kuba/projects/sealcode/sealious/sealious
> mocha --recursive tests

/home/kuba/projects/sealcode/sealious/sealious/lib/subject/predefined-subjects/me-subject.js:12
MeSubject.prototype = Object.create(Sealious.Subject.prototype);
                                                    ^

TypeError: Cannot read property 'prototype' of undefined
    at Object.<anonymous> (/home/kuba/projects/sealcode/sealious/sealious/lib/subject/predefined-subjects/me-subject.js:12:53)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:456:32)
    at tryModuleLoad (module.js:415:12)
    at Function.Module._load (module.js:407:3)
    at Module.require (module.js:466:17)
    at require (internal/module.js:20:19)

Could you look into it? Does it fail for you, as well?

[adwydman]

I cannot recreate this error, as the test pass:

$ npm test

> sealious@0.7.11 test C:\Users\anwi\Desktop\sealious
> mocha --recursive tests

  AccessStrategy.Logged_In
    √ returns the name of the access strategy
    √ resolves the checker function
    √ rejects the checker function

  AccessStrategy.Noone
    √ returns the name of the access strategy
    √ rejects the checker_function
    √ checks item_sensitive atribute

(...)

[kuba-orlik]

Hmmm... I've removed the node_modules directory and reinstalled all of the dependencies. Few checks:

• are you on the 2c37b780ce4d8eb3ad8717d9e1609dcf4ad963ab commit when testing?
• could you remove the node_modules directory and reinstall all dependencies?
• what operating system are you using for the tests?
• what Node version are you using for tests?
• what npm vestion are you on?

[adwydman]

Hm, after deleting node_modules, I have the same error.

[kuba-orlik]

Please merge the references branch to rewrite_new_function_to_object it will bring the locreq changes, possibly helping with those cyclic dependencies.

[adwydman]

Tests pass after merging references to this branch.

[coveralls]

Coverage decreased (-1.6%) to 51.301% when pulling edeafde57aa90747304b389456291eb11d87a92b on rewrite_new_function_to_object into e41dca416a2cafb5cd104c8316b13fc42f77fca4 on references.