-
Notifications
You must be signed in to change notification settings - Fork 12
Lab 1 - Britt #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Lab 1 - Britt #12
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| 'use strict'; | ||
|
|
||
| const greeting = require('./lib/greet.js'); | ||
|
|
||
| greeting.greet(process.argv[2]); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| 'use strict'; | ||
|
|
||
| module.exports = exports = {}; | ||
|
|
||
| exports.greet = function(name) { | ||
| console.log(`hello ${name}`); | ||
| if (!arguments.length) throw new Error('name not provided'); | ||
| return `hello ${name}!`; | ||
| }; | ||
|
|
||
| console.log('hello', process.argv[2]); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you are just exporting a single function in this case, module.exports = { } would be more appropriate. You want to use module.exports = exports = { } when you are exporting multiple methods. For instance, if you had another method attached to the exports object in this file, then the way you are using module exports on line 3 would be just fine :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For instance, you could have attached another method to the exports object that handles the bonus and within that method, declared a var for process.argv[2] and then pass that as the argument. Then, in index.js, you just invoke that method and keep the original greet method that accepts a name as parameter. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| 'use strict'; | ||
|
|
||
| const greeting = require('../lib/greet.js'); | ||
| const assert = require('assert'); | ||
|
|
||
| describe('Greeting Module', function() { | ||
| describe('#greet', function() { | ||
| it('should return hello and name if input in command line', function() { | ||
| var inputName = process.argv[2]; | ||
| let result = greeting.greet(inputName); | ||
| assert.ok(result === `hello ${inputName}!`, 'but not equal to hello and input name.'); | ||
| }); | ||
| it('should throw an undefined if name argument is not passed in command line', function() { | ||
| assert.throws(function() { | ||
| greeting.greet(); | ||
| }, 'error not thrown'); | ||
| }); | ||
| }); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice work around for testing process.argv[2] As I'm sure you may have discovered, mocha doesn't accept command line arguments, so in this case, the better test would have been to check for a truthy value for inputName. Testing for a command line argument in mocha will always be undefined.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good to know! |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad to see you make use of process.argv[2] to solve the bonus!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could have also stored process.argv[2] in a variable and pass it as an argument to a separate function, see below for further elaboration on this!