source: branches/FACT++_scripts_refactoring/explicit_changelog@ 18481

18.06.2016:
-----------

Start of Refactoring.

The entire FACT++/scripts folder contains ~5k lines of JS code, hardly documented at all. Yes, there are comments, but this is mostly commented out code, not anything that tries make the design choices clear to the reader.But this is not what worries me most. 

Mostly I am worried about, what I like to call "global include with side-effects".
AFAIK in ECMAScript 5 there is nothing like an include command, but in our FACT++ JS dialect, we have include(). From what I infer, this function will simply insert the mentioned script at its own position. Much like the C-preprocessor would execute a #include. 

This means, by including another script, one does not only make new functions and objects available, but one can also (unknowingly) execute code. Even more, this inserted script is inserted into the global scope (mostly), so it has access to all the variables and functions, previously defined and can alter them. So that after an include(), one cannot be sure about the state ones variables. I can imagine, a nice source of error would be the following:

As script works nicely, but somebody needs additional output, so a new variable is created in the top of Main.js:

    var i_am_already_used_inside_an_included_script = some_value;

Further down the line, there is an include(), which alters the value of "i_am_already_used_inside_an_included_script", because it happened to use the same variable name.

Further down, our new variable has a different value, and we just don't see why. 

#-------------------------------------------------------------

So to sum it up. I would like it best, when.

A) There are no scripts to be included, that have side-effects.
    I.e. All scripts, which are included into the Main.js or any other main-like script, should only create new functions, or reference types (aka classes), but never call any function or instanciate new objects.

B) Even better: Scripts, which are written, to be included into a main-like script, should simply create their own scope. If they have their own scope, of course they may create instances of new classes, or any other variable, which stays bound to the 
scope of the script. They still *should not* execute any code, I'd say.

Example:
    # great_functions.js

    var great_functions = {

        scoped_variable1 : "foo",

        great_function1 : function(parameter1){
            console.out(parameter1);
        },
        
        great_function2 :function(){
            console.out(this.scoped_variable1);
        },
    };

Use the functions from great_functions.js like:

    include("great_functions.js");
    great_functions.great_function1("foo");     // --> foo
    great_functions.scoped_variable1 = "bar";
    great_functions.great_function2();          // --> bar
    

C) If one wants to create a lot of funtions, which should go into one single scope, like a library for example. I'd suggest to do it like this:

Create a folder for the library and inside that folder create a file called like the library as well. 

new_lib/
   |
   |----new_lib.js
   |
   |----part1.js
   |
   |----part2.js

Let new_lib.js create the scope and include all parts of the library inside new_lib.js:

    # new_lib.js

    var new_lib = {};
    include("part1.js");
    include("part2.js");

Then inside the parts of the library (names can be chosen arbitraryly of course.) attach the new functions or reference types to the namespace created in new_lib.js:

    # par1.js

    new_lib.a_function = function(){
        // do something astonishing!
    };


Of course all this heavily relies on convention, and not to break the convention. But I think, it would work. In the main-like script, which wants to use a_function() just do:

    # main_like.js

    include("new_lib/new_lib.js");
    new_lib.a_function();



# --------------------------------------------------------------

So there we are ... I am now on the mission of scoping scripts and removing side-effects. 

I'll start with side-effects, because they are much worse, than missing scope.

So I'll start with "CheckFTU.js", which is included in "Startup.js"

Okay, took me 5 minutes. not bad. 

next is:
"CheckUnderflow.js"

Okay so, this is more tricky. CheckUnderflow.js includes 4 more scripts.
    CheckStates.js is included in CheckUnderflow.js *and* in Startup.js

I think, it is time to invent something like include-guards. So, when a library is included another time, it checks, weather it was already included. Maybe like this:

    # library.js
    // this should be the global context, *if* include('library.js')
    // was called in a global context. 
    if (!('library' in this)){
        var library = {
            some_func : function(){

            },
            another_func : function(){

            },
        };
    }
    //maybe even: 
    else{
        console.out("multiple include of 'library.js' ");
    }

So I add, this new "include guard" to CheckFTU.js quickly.

Still we need to decide, what we want to do, when include() is called
inside a libraries scope. Currently it would be included, into that namespace, and stay there. Which is probably just fine. So... 
let's got for it, and see how it works.

# ------------------------------------------------------------------------
So CheckUnderflow.js is done. Now I look at: CheckStates.js which is included by CheckUnderflow.js.

Okay, I'm done inside CheckStates.js, now I need to see, how the functions are called.

While doing CheckStates.js, I realized, that functions which are part of a scope now, cannot refer to each other directly, as they could do before.
Now when once function inside a library wants to call another function, it must refer to it, using this. This is a bit more to write, but it makes it also clear, where the function, which is called, comes from. So for the sake of expliciteness. I'll do it.

Now I do Hist1D.js and Hist2D.js. Also I must say, I found a little library, being defined inside CheckUnderflow, which is the Func object.

So I will create a Func.js for this.
# ----------------------------------------------------------------------

Okay now I look at takeRun.js as it is included in CheckUnderflow.
I had to create a new function, which would handle the attachment of 
the inchange function to the FACT_CONTROL/INCOMPLETE subscription.
I don't know a good name for this, but I think I wrote it correctly.

I don't like that the new function takeRun.attachFadControlIncompleteOnChange 
has a member named takeRun.attachFadControlIncompleteOnChange.incomplete.

I also don't like, that in CheckUnderflow, I call this new function,
which will attach a callback function to the FACT_CONTROL/INCOMPLETE subscription.
But when reading the code, one can hardly understand that.

I mean, this is just like when takeRun.js was included before.
Nobody could have even guessed, that there was a callback function attached to 
a Subscription instance made just before takeRun.js was included. 

Okay that's it for the moment. Next is probably CheckUnderflow itself,
since it is a kind of library itself.

# ----------------------------------------------------------------------

Okay so I looked at CheckUnderflow.js. It was a pretty large mess, Which I broke up into
11 separate functions, none of them longer than one page. 
But there was no possibility to reuse anything. So the code in total got larger.
Well, we will see if there is any possibility to reuse later, when refactoring is done.

At least, I could imagine, that some of the function would in principle be testable,
while I have no idea, how to test anything, which is dim dependent, without the overhead 
of writing a total mock up.

Next up is: Statup.js, because this uses CheckUnderflow.

Then I will look at all these handlers. Let's see.

# ----------------------------------------------------------------------

I just remembered, I should mention what I donÄt like about Hist1D.js and Hist2D.js
(I mean apart from the fact, that this code would not be needed, 
when we had used Python+PyDim instead of Javascript and a custom made JS interpreter.)
I don't like that Hist1D() and Hist2D() *claim* to be a constructors, but in fact they are not.

If you did:
    var h = Hist1D.Hist1D(100, 0, 100);
    console.out(h instanceof Hist1D);
you would get a 'false'. 

Also the fact, that one always has to call constructors with new, does not apply for Hist1D or Hist2D, why?
In strict mode, not calling a constructor with new, will even cause an exception. But calling Hist1D() with or without new, does not make a difference at all.

Should be changed.

# ----------------------------------------------------------------------