#139 new
nickg

Dead code/apis for Envjs?

Reported by nickg | March 27th, 2010 @ 01:23 PM

in the src/platform I found the following that don't appear to be used anymore.

Anyone using these? Or do we need a 'deprecated.js' file for core and rhino?

There are a lot of "bits and pieces" like Envjs.version and whatnot that I didn't investigate.


src/platform/core/console.js:

I think these are now superseded by the console api anyways.

/**   
 * Constants providing enumerated levels for logging in modules
 */
Envjs.DEBUG = 1;
Envjs.INFO = 2;
Envjs.WARN = 3;
Envjs.ERROR = 3;
Envjs.NONE = 3;

No idea what the profile module is for. Rhino shouldn't dupe the default version anyways.

src/platform/core/profile.js src/platform/rhino/profile.js

/**
 *                                                                              
 * @param {Object} options
 */
Envjs.profile = function(options){ throw new Error(this); };

src/platform/core/rhino/timer.js:

Defined only in rhino, but not in core, and not used.

/**                                                                         
 * provides callback hook for when the system exits                       
 */
Envjs.onExit = function(callback){
    var rhino = Packages.org.mozilla.javascript,
        contextFactory =  __context__.getFactory(),

//... etc.....

Comments and changes to this ticket

  • nickg

    nickg March 27th, 2010 @ 01:55 PM

    found another. Envjs.spawn seems unused and not in core

    src/platform/rhino/timer.js

    try{
        Envjs.sync = sync;
        Envjs.spawn = spawn;
    } catch(e){
        //sync unavailable on AppEngine                                                                                                                         
        Envjs.sync = function(fn){
            //console.log('Threadless platform, sync is safe');                                                                                                 
            return fn;
        };
    
        Envjs.spawn = function(fn){
            //console.log('Threadless platform, spawn shares main thread.');                                                                                    
            return fn();
        };
    }
    
  • Thatcher

    Thatcher March 27th, 2010 @ 03:13 PM

    Some clean up is good, but there are a couple reasons I've kept them around, if only to avoid regression as folks try to port to newer releases.

    Heres my general thoughts:

    Envjs.DEBUG etc are just constants and I'm not really sure how they could get in the way of adding support for another js engine. I've actually been working on porting a full featured category logging framework to a stand alone implementation so we can add some better logging inside envjs. The 5 levels alone were really insufficient to be useful for looking under the hood when you need to.

    Envjs.profile is basically unused, early on it was used to apply AOP 'arounds' to selected portions of the api to see where envjs spent most of its time and to sniff out innefficient code. It was only used internally and hasnt been used for along time so it can go.

    Envjs.onExit is a hook. Its not required by a platform but implemented for the rhino platform. It is not used internally but its a nice hook to have and I use it in projects that use Envjs. It should not affect the ability to support another platform since its not a required interface.

    Envjs.sync and Envjs.spawn are used the the xhr and timer modules to provide platform implementations for performing syncronized operations when another thread is running. I don't believe they can be removed.

    Nice work lately nick, It's been nice to be able to catch up on some of my back logs.

  • nickg

    nickg March 27th, 2010 @ 04:21 PM

    Oh I'm happy to keep them around, just figuring out what may or may not need to be ported, and put some of these notes in code.

    I see 'sync' is used (a lot), but i did not see spawn used. I assume it's to spawn another process. I would probably separate those into separate blocks, but whatev. Not critical.

    I found some other that where defined in rhino but not in core (of the top of my head 'proxy' is one)

    I'm sure I'l have more questions as I dig around.

  • Thatcher

    Thatcher March 28th, 2010 @ 10:07 AM

    Yeah I know its confusing, I'm not happy with the way the plaftorm
    implementation stuff is broken up and its not clear what is required and
    what is optional. I feel like it might be better to go back a single file
    for the 'core' and a single file for each platform implementation. What do
    you think? Have you found it terribly confusing (I would expect it to be
    confusing at this point)?

  • nickg

    nickg March 28th, 2010 @ 12:50 PM

    Hi,

    I think single file is fine. I don't see really the value in the multiple file approach to be honest. Plus, some of the items are debatable which file they should go in.

    Hmmm another idea, is to have a separate object called "Envjs.external" for all callouts that are required. This way stuff like Envjs.version is fine as is, but Envjs.external.sleep would be platform-specific.

    --nickg

  • Steven Parkes

    Steven Parkes March 28th, 2010 @ 04:33 PM

    I tend to agree with the external/platform object as a switch point. I'd actually prefer to derive (in the javascript sense) the platform object from the base version. That would seem to give the most flexibility.

  • nickg

    nickg March 28th, 2010 @ 05:14 PM

    yeah using a normal object model would make sense and prob be more self-documenting.

    chris what do you think?

  • Thatcher

    Thatcher March 29th, 2010 @ 03:28 PM

    totally down for that. the current organization is a head ache. I'm never
    afraid to admit I make mistakes because I do sleep in my own bed after I
    make it and often find myself uncomfortable ;)

  • Steven Parkes

    Steven Parkes March 29th, 2010 @ 03:43 PM

    Way too modest. A little glitch on so much good work. And a lot of people use it, successfully. But none that take on anything like implementing a browser.

    I'm dying to get back on the main fork. I want all the good work. It's gonna have to wait until after mid-April, but after that ...

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

a javascript browser environment

People watching this ticket

Pages