Chemical scheme calculator

Question

This code implements an algorithm for calculation of chemical schemes. This code looks strange but now I have no idea how to refactor it. Can you give your feedback?

eps=0.001;

s1=new Source(1000,0.17);
mx=new Mixer(1000);
mb=new Mem(0.15,0.95);
sp=new Splitter(0.80);
s2=new Sink();
s3=new Sink();

mx.in1=s1.out1;
mx.in2=sp.out2;
mb.in1=mx.out1;
sp.in1=mb.out2;
s3.in1=sp.out1;
s2.in1=mb.out1;

for (i=0;i<50;i++){
    mx.calc();
    mb.calc();
    sp.calc();
}

function Stream(v,c){
    this.v=v||null;
    this.c=c||null;
    this.selfCheck=false;
    this.Show=function(){//how to add default values?
        return "volume="+this.v+",conc="+this.c+",selfCheck:"+this.selfCheck+"; ";
    }
}

function Source(v,c){
    this.out1=new Stream(v,c);
    this.calc=function(){};
}
function Sink(){
    this.in1=null;
    this.calc=function(){};
}

function Mixer(fixedV){
    this.fv=fixedV;
    this.in1=null;
    this.in2=null;
    this.out1=new Stream();
    this.calc=function(){
        this.out1.v=this.fv;//||this.in1.v+this.in2.v;
        this.in2.v=this.in2.v||0;
        this.in2.c=this.in2.c||0;
        this.in1.v=this.out1.v-this.in2.v;
        this.out1.c =(this.in1.v*this.in1.c+this.in2.v*this.in2.c)/this.out1.v;
        this.out1.selfCheck=Math.abs
        ((this.in1.v*this.in1.c+this.in2.v*this.in2.c)-(this.out1.v*this.out1.c))<eps;
    }
}

function Splitter(kS){
    this.in1=null;
    this.ks=kS||0.05;
    this.out1=new Stream();
    this.out2=new Stream();
    this.calc=function(){
        this.out1.v=this.in1.v*(1-this.ks);
        this.out2.v=this.in1.v*(this.ks);
        this.out1.c=this.in1.c;
        this.out2.c=this.in1.c;
    }

}

function Mem(kV,kC) {
    this.kv = kV||0.15;
    this.kc = kC||0.95;
    this.in1 = null;
    this.out1 = new Stream();
    this.out2 = new Stream();
    this.calc = function () {
        this.out1.v = this.in1.v * this.kv;
        this.out1.c = this.in1.c * (1 - this.kc);
        this.out2.v = this.in1.v * (1 - this.kv);
        this.out2.c = (this.in1.v * this.in1.c - this.out1.v * this.out1.c) / this.out2.v;
        this.out1.selfCheck = this.out2.selfCheck = Math.abs
        (this.in1.v * this.in1.c - (this.out1.v * this.out1.c + this.out2.v * this.out2.c)) < eps;

    }
}

Answer 1

A couple of format rules like:

• Is a good practice declare first and then use it. Is not a big deal, but is a good practice.

• Try to use, Pascal Case for constructors, and camelCase for methods.

• You can replace some repeated function with inheritance javascript

Also, try to don´t abuse of the this. In javascript is a little tricky and can be changed depending of the context. For default values use

var something = original Value || default Value;

also, last thing most important for your calculator, take a look to the javascript floating points troubles maybe you can get wrong results.

Answer 2

I was going to go through your code and rewrite it but some of the names are very ambiguous so I do not want to in case I break something.

Also a few of these points are duplicated from @vmariano.

• Focus on using names that make sense. v and c do not make much sense to me at all. They do not mean anything to me. Perhaps they are part of a chemical formula, but I am not a chemist and a vast majority of programmers also will not be. Imagine that the code you are writing is being shown to someone who only knows how to program in the language you are programming in, and nothing else. The code still has to make sense to them.
• You should avoid the liberal use of this. It can be rebound function call to function call very easily. Whilst some this is OK, and even necessary, it is better to at least keep it consistent. To give you an idea, the this in your Stream constructor could refer to a different this to the this in the Stream.Show method.
• In general, PascalCase for constructors - camelCase for everything else.
• function Stream is fine and lets you use the function before the function itself is declared.. this is known as a function declaration. However this can make the code harder to read because you place all of the functions at the bottom. Consider using a combination between function express and declaration - var Stream = function Stream(). You will have to place this before your working code but it is a lot cleaner to read and you still maintain the name of the function in console.log or stacktraces.
• Better yet, your code right now does a lot. How about you move each constructor into it's own file and require them (or load them in the right order/use browserify/requirejs if you're writing this for the browser)?
• Avoid assigning null to values by default, it will cause your code to break. Instead, assign sensible defaults or nothing at all. Values that are not assigned have the special value undefined.
• Instead of using new Stream() in your functions, consider passing in a stream into the constructor itself; this will decouple your code.
• Your Math.abs call at the end of the code is confusing. Keep it on one line. IF the line is too long, then change the names of variables to make them shorter.