Skip to content
This repository has been archived by the owner on Sep 23, 2021. It is now read-only.

Why InitConn in utils? #18

Open
jpaulm opened this issue Feb 13, 2015 · 11 comments
Open

Why InitConn in utils? #18

jpaulm opened this issue Feb 13, 2015 · 11 comments
Labels

Comments

@jpaulm
Copy link
Owner

jpaulm commented Feb 13, 2015

@ComFreek - I like the way you have split up index.js, and I have updated the Readme to reflect that getElementWithSmallestBacklog is in utils, but I am wondering why InitConn is also in utils, as it is not exposed to components (although it is used by initialize)... Just wondering! Thx.

@ComFreek
Copy link
Collaborator

I have just put lone functions (except for run and run2, I think) from index.js into utils.js.
You are right, it shouldn't belong to utils.js! However, I am not quite sure whether InitConn is a good name. It gets only once used:

module.exports.initialize = function(proc, port, string) {
  var inport = new InputPort(queue);
  inport.name = proc.name + "." + port;
  inport.conn = new Utils.InitConn(string);
  proc.inports[proc.inports.length] = [proc.name + '.' + port, inport];
};

We either initialize an object or create it using new. new Utils.InitConn feels somehow wrong to me. What is its use case? Why can't we use the Connection class which already exists?

BTW, am I right to assume that proc.inports[proc.inports.length] = [proc.name + '.' + port, inport]; should push to the array? If so, proc.inports.push(...) is a lot simpler.


EDIT:
I am not particularly happy with an utils file in which everything one fails to categorize ends up. Maybe we should make a folder utils instead and put functions in individual files. Or is this an overkill?

@jpaulm
Copy link
Owner Author

jpaulm commented Feb 13, 2015

An InputPort is connected either to a Connection or an InitConn, and
receive and a few other functions have to test which the InputPort is
connected to. I see Connection as connecting two processes, whereas
InitConn connects a String with a Process. However, I agree that InitConn
is a bit awkward - how about if we add a field to Connection (to point at
the String) and have receive and the other functions simply test whether
that field is null or not...?

Regards,

Paul

On Fri, Feb 13, 2015 at 8:13 AM, ComFreek [email protected] wrote:

I have just put lone functions (except for run and run2, I think) from
index.js into utils.js.
You are right, it shouldn't belong to utils.js! However, I am not quite
sure whether InitConn is a good name. It gets only once used:

module.exports.initialize = function(proc, port, string) {
var inport = new InputPort(queue);
inport.name = proc.name + "." + port;
inport.conn = new Utils.InitConn(string);
proc.inports[proc.inports.length] = [proc.name + '.' + port, inport];
};

We either initialize an object or create it using new. new Utils.InitConn
feels somehow wrong to me. What is its use case? Why can't we use the
Connection class which already exists?


Reply to this email directly or view it on GitHub
#18 (comment).

@ComFreek
Copy link
Collaborator

If I understand you correctly, an InitConn is basically a Process with 0 inports, 1 outport, and whose function returns a constant/pre-defined string. Is that correct?
Another option would be to have a generic and abstract GenericProcess class and two subclasses, namely (Normal)Process and StringConstantProcess.

Where is that receive function you mentioned? Unfortunately, I could not find the place where the contents of an InitConn are read neither.

@ComFreek
Copy link
Collaborator

@jpaulm I don't know whether you are aware of this, but there are two users with which GitHub identifies your commits: https://github.com/jpaulm/jsfbp/commits/ That's probably because you are using two different names.

@jpaulm
Copy link
Owner Author

jpaulm commented Feb 13, 2015

I think the two users are jpaulm and J. Paul Morrison - right? It seems to
depend on whether I made the change by editing on Github (my usual action
with Readme) or whether I made them on my desktop and did a push... Could
this cause problems?

Regards,

Paul

On Fri, Feb 13, 2015 at 11:29 AM, ComFreek [email protected] wrote:

@jpaulm https://github.com/jpaulm I don't know whether you are aware of
this, but there are two users with which GitHub identifies your comits:
https://github.com/jpaulm/jsfbp/commits/master


Reply to this email directly or view it on GitHub
#18 (comment).

@ComFreek
Copy link
Collaborator

Exactly, you could try setting your local Git username and mail.

Execute these commands:

git config --global user.name "jpaulm"
git config --global user.email "..."

(You can verify these settings by leaving out the last part, i.e. not passing a new value.)

@jpaulm
Copy link
Owner Author

jpaulm commented Feb 13, 2015

I tend to resist calling InitConn a Process as it is not active - it is
strictly passive, but since it is under the covers, I don't object to
rolling that function into Connection... What's your feeling?

Receive is in InputPort, and there are 5 references to it in index.js.

Regards,

Paul

On Fri, Feb 13, 2015 at 11:26 AM, ComFreek [email protected] wrote:

If I understand you correctly, an InitConn is basically a Process with 0
inports, 1 outport, and whose function returns a constant/pre-defined
string. Is that correct?
Another option would be to have a generic and abstract GenericProcess
class and two subclasses, namely (Normal)Process and StringConstantProcess
.

Where is that receive function you mentioned? Unfortunately, I could not
find the place where the contents of an InitConn are read neither.


Reply to this email directly or view it on GitHub
#18 (comment).

@jpaulm
Copy link
Owner Author

jpaulm commented Feb 13, 2015

Done! Thanks!

Paul

On Fri, Feb 13, 2015 at 12:00 PM, ComFreek [email protected] wrote:

Exactly, you could try to set your local Git username and mail.

Execute these commands:

git config --global user.name "jpaulm"
git config --global user.email "..."

(You can verify these settings by leaving out the last part, i.e. not
passing a new value.)


Reply to this email directly or view it on GitHub
#18 (comment).

@ComFreek
Copy link
Collaborator

I see!
I think moving InitConn into Connection doesn't have much advantage (=code complexity) to it if we are still forced to check both cases (InitConn/Real connection) in InputPort#receive. Would it make sense to let Connection handle the cases? May a Connection handle the creation of an IP at all?

@tlrobinson What do you think?

Nonetheless, we should definitely document the code with JSDoc syntax, which hopefully alleviates the confusion curious readers might have.

@jpaulm Have you done any recent changes to InputPort? I remember having cleaned up the code (adding braces, fixing indentation). This got somehow reverted, e.g. here. Or did I leave that file out? Indeed 😄

@jpaulm
Copy link
Owner Author

jpaulm commented Feb 13, 2015

On Fri, Feb 13, 2015 at 12:23 PM, ComFreek [email protected] wrote:

I see!
I think moving InitConn into Connection doesn't have much advantage (=code
complexity) to it if we are still forced to check both cases (InitConn/Real
connection) in InputPort#receive. Would it make sense to let Connection
handle the cases? May a Connection handle the creation of an IP at all?

I'm open to suggestions! My code tends to be rough and ready!

@tlrobinson https://github.com/tlrobinson What do you think?

Nonetheless, we should definitely document the code with JSDoc syntax
http://usejsdoc.org/, which hopefully alleviates the confusion curious
readers might have.

How much do you document? Any examples?

@jpaulm https://github.com/jpaulm Have you done any recent changes to
InputPort? I remember having cleaned up the code (adding braces, fixing
indentation). This got somehow reverted, e.g. here

return null;
.
Or did I leave that file out?

Not that I know of. I merged @tlbrobinson 's code first, then yours a day
later...

Regards,

Paul


Reply to this email directly or view it on GitHub
#18 (comment).

@ComFreek
Copy link
Collaborator

I've partly documented InputPort.js for demonstration purposes:
https://github.com/ComFreek/jsfbp.git

You can execute these commands to view the resulting documentation:

  1. git clone https://github.com/ComFreek/jsfbp.git
  2. git checkout code-documentation
  3. npm install
  4. npm run-script doc
  5. Open out/index.html

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants