I am trying to figure out a bug that i've been running into in my code. I wrote out a simple example that replicated the bug using classes a, b and a entry file c.
Where is my code going wrong?
Edit
Many of the answers are concerning my code structure! I would like to clarify the functionality that is required.
a: needs a static instance of b. Therefore, A needs to require B.
b's functions (none static): only accept objects that are an
instanceof a
. Therefore, B needs to require A.
Specifically, I have a "Phrase" class (a) and a "Parser" class (b). Parser only accepts phrases. Phrase parses its self on creation using its static parser instance.
a.js
'use strict';
let b = require('./b.js');
class a {
constructor() {
}
}
new b();
module.exports = a;
b.js
'use strict';
let a = require('./a.js');
class b {
constructor() {
}
}
module.exports = b;
c.js
'use strict';
let b = require('./b.js');
new b();
run: node c.js
error:
C:\code\a.js:11
new b();
^
TypeError: b is not a function
at Object.<anonymous> (C:\code\projects\elegance\data-frog\tag\datastructures\a.js:11:7)
at Module._pile (module.js:413:34)
at Object.Module._extensions..js (module.js:422:10)
at Module.load (module.js:357:32)
at Function.Module._load (module.js:314:12)
at Module.require (module.js:367:17)
at require (internal/module.js:16:19)
at Object.<anonymous> (C:\code\projects\elegance\data-frog\tag\datastructures\b.js:3:9)
at Module._pile (module.js:413:34)
at Object.Module._extensions..js (module.js:422:10)
I am trying to figure out a bug that i've been running into in my code. I wrote out a simple example that replicated the bug using classes a, b and a entry file c.
Where is my code going wrong?
Edit
Many of the answers are concerning my code structure! I would like to clarify the functionality that is required.
a: needs a static instance of b. Therefore, A needs to require B.
b's functions (none static): only accept objects that are an
instanceof a
. Therefore, B needs to require A.
Specifically, I have a "Phrase" class (a) and a "Parser" class (b). Parser only accepts phrases. Phrase parses its self on creation using its static parser instance.
a.js
'use strict';
let b = require('./b.js');
class a {
constructor() {
}
}
new b();
module.exports = a;
b.js
'use strict';
let a = require('./a.js');
class b {
constructor() {
}
}
module.exports = b;
c.js
'use strict';
let b = require('./b.js');
new b();
run: node c.js
error:
C:\code\a.js:11
new b();
^
TypeError: b is not a function
at Object.<anonymous> (C:\code\projects\elegance\data-frog\tag\datastructures\a.js:11:7)
at Module._pile (module.js:413:34)
at Object.Module._extensions..js (module.js:422:10)
at Module.load (module.js:357:32)
at Function.Module._load (module.js:314:12)
at Module.require (module.js:367:17)
at require (internal/module.js:16:19)
at Object.<anonymous> (C:\code\projects\elegance\data-frog\tag\datastructures\b.js:3:9)
at Module._pile (module.js:413:34)
at Object.Module._extensions..js (module.js:422:10)
Share
Improve this question
edited Mar 28, 2016 at 2:41
Michael Petrochuk
asked Mar 28, 2016 at 2:03
Michael PetrochukMichael Petrochuk
4971 gold badge5 silver badges20 bronze badges
3
- Which version of Node are you using? – Akshat Mahajan Commented Mar 28, 2016 at 2:06
- Using version v5.9.0 – Michael Petrochuk Commented Mar 28, 2016 at 2:06
- Looking at the edit, for what i know, the only way to do what you need is really to use a third class as i illustrate on my answer. – pietrovismara Commented Mar 28, 2016 at 19:46
3 Answers
Reset to default 3You are recursively calling each module from inside the other, so when you import b in the a module, it is not ready yet.
You should not require a.js inside the b module.
If for some reason you need to do it, refactor your code because this is an antipattern.
You could use a third module which require a and b like this:
const A = require('a'); //Export the class, not the instance
const B = require('b'); //Export the class, not the instance
var a = new A();
var b = new B();
a.b = b;
This way you are also sure that a and b are instances of their respective classes, or if you prefer you can still use the required classes for instanceof
.
Also, if I can suggest, use uppercase naming for your Classes.
Update
Based upon your edit, my immediate suggestion is that you continue to use class Phrase
if you choose to keep it, specifically because you like having the ability to log the type of an object...
...but to use that constructor as nothing more than a struct
that you would find in other languages.
Your Parser
can know about Phrase
s, given that it parses them, but there's really 0 harm in having the parser accept duck-typed entries, as the difference in flexibility is large, if you check for the interface your method wants (fields on the struct that you actually use), versus checking for a strict instance.
A quick example may be loading a list of phrases from a web service. Do you really need to instantiate every line as a phrase, before parsing them?
Here's an example of how you might handle the case in which you have a low-level struct, which doesn't really need a factory for DI (because it's so low-level), but rather, could be directly imported into the Parser
module.
// phrase.js
class Phrase {
constructor (content = "") {
this.content = content;
}
static from (phrase) {
return new Phrase(phrase.content);
}
static isPhrase (phrase) {
return phrase && phrase.content !== null && phrase.content !== undefined;
}
}
export default Phrase; // or module.exports
// parser.js
import Parser from "./parser"; // or require, whatever floats your boat
class Parser {
static parse ( input = "" ) {
const preparedContent = Phrase.isPhrase( input )
? Parser.prepareContent( input.content )
: Parser.prepareContent( input );
return new Phrase(content);
}
static prepareContent ( input = "" ) {
// ...
}
parse (content) {
return Parser.parse(content);
}
}
export default Parser;
I can now make an instance of Parser
or just statically use it.
I can hand it raw input, or I can hand it a Phrase, and it will do the right thing.
Moreover, I'm not relying on Parser to be able to define what Phrase is. Parser is relying on a Phrase builder to know what Phrase is.
Parser is always returning a new instance of a "parsed" Phrase, but its input is now more relaxed. As well, based on what is ing in, you can now do more-sane things with your input (ie: provide good defaults, like safely returning an empty phrase (where phrase.content
in my example world might be an empty string), rather than returning null
or throwing, based on an instanceof).
Have a look at an answer which I provided elsewhere, for further details (and check the question to see where he hit a similar roadblock exactly one stop beyond where you are, currently).
ES6 modules and inheritance
The answer, though, is that you should not be relying on circular dependencies, which will do terrible, terrible things to your codebase.
If you really, really, well and truly need to do this (you don't), you should be using a Factory
to ensure that the correct instances are built/used, and injecting the factory into your application, rather than the classes themselves.
If you are seriously depending, wholesale, upon the typeof
or instanceof
functionality to ensure the sturdiness of your application -- especially in such a way that the check requires circular references, then your application is not sturdy in the first place
Your classes should not need to know about one another, and when you delegate their construction to factories, if that's the path you choose to take, then the classes should not know about the factory, either.
Might be plicating more this then it might be.
Check out this simple class of a light.
class Light {
constructor(state = false) {
this.state = state;
}
on() {
this.state = true;
return this.state;
}
off() {
this.state = false;
return this.state;
}
}
const myLight = new Light();
myLight.on()
You can see that the state is defined in the constructor.
Hope this helps.