-
Notifications
You must be signed in to change notification settings - Fork 45
Homework#3_102502520 #31
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?
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,11 @@ | ||
| .DS_Store | ||
| *.rej | ||
| *.log | ||
| *.orig | ||
| *.pyc | ||
| *.swp | ||
| *.swn | ||
| *.swo | ||
| .svn | ||
| *~ | ||
| node_modules/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # Homework | ||
|
|
||
| Refactor code in this directory. | ||
| To try the result in a simple way: | ||
|
|
||
| python -m SimpleHTTPServer | ||
|
|
||
| And open your browser with | ||
|
|
||
| http://127.0.0.1:8000/ | ||
|
|
||
| ## How to get points | ||
|
|
||
| * You need at least address all the Tier 1 issues to get 4 points | ||
| * You will get the extra 3 points from addressing Tier 2 issues | ||
| * However, you will NOT get any points from the Tier 2, | ||
| if you fail to address ANY single issue of Tier 1 | ||
| * You need at least the 4 points from Tier 1 to pass this class | ||
| * You can add or remove some code from the example. However, make sure every change you make is with clear intentions | ||
|
|
||
|
|
||
| ## Tier 1 requirements | ||
|
|
||
| * Bind this in Event handlers | ||
| * Deal with object properties w/ constructor & prototype | ||
| * Deal with asynchronous flows with Promise | ||
| * Write your first test case for a pure function (as the last test of |test-list.js| file shows) | ||
|
|
||
| ## Tier 2 requirements | ||
|
|
||
| * Implement functions with closure when it's necessary | ||
| * Avoid using closure when it shouldn't appear | ||
| * Write meaningful comments in JSDoc format | ||
| * (reference: https://en.wikipedia.org/wiki/JSDoc#Example) | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # About Test | ||
|
|
||
| To install dependencies: | ||
|
|
||
| npm install | ||
|
|
||
| To run it: | ||
|
|
||
| ./node_modules/karma/bin/karma start |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| 'use strict'; | ||
|
|
||
| (function(exports) { | ||
|
|
||
| var ContentManager = function(){ | ||
| this._wrapper = null; | ||
| }; | ||
|
|
||
| ContentManager.prototype = { | ||
|
|
||
| start() { | ||
| window.addEventListener('note-open', function(event) { | ||
| var note = event.detail; | ||
| this.resetWrapper(); | ||
| this.drawNote(note); | ||
| }).blind(this)); | ||
| } | ||
| }; | ||
| resetWrapper() { | ||
| this._wrapper.innerHTML = ''; | ||
| } | ||
|
|
||
| drawNote(note) { | ||
| var title = note.title; | ||
| var h = document.createElement('h2'); | ||
| h.textContent = title; | ||
| var passages = note.passages; | ||
| var buff = document.createDocumentFragment(); | ||
| passages.forEach(function(passage) { | ||
| var p = document.createElement('p'); | ||
| p.classList.add('note-passage'); | ||
| p.textContent = passage; | ||
| buff.appendChild(p); | ||
| }); | ||
| this._wrapper.appendChild(h); | ||
| this._wrapper.appendChild(buff); | ||
| } | ||
|
|
||
| exports.ContentManager = ContentManager; | ||
|
|
||
| })(window); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| [ | ||
| { "title": "Inside Japan’s Disposable Housing Market", | ||
| "passages": [ | ||
| "In a culture obsessed with newness, no one wants a 'used' home—which makes the Japanese real estate market almost unrecognizable to an American.", | ||
| "The 58-year-old writer, born in a sleepy bayside suburb on the north shore of New York’s Long Island, had lived in the country for more than 15 years. He and his wife, Masako Tsubuku, 57, scoured the home market for something affordable and livable, only to find dilapidated houses that even the real estate agents expected them to demolish and re-build upon." | ||
| ]}, | ||
|
|
||
| { "title": "How coffee loves us back", | ||
| "passages": [ | ||
| "Coffee, said the Napoleon-era French diplomat Talleyrand, should be hot as hell, black as the devil, pure as an angel, sweet as love.", | ||
| "Bach wrote a cantata in its honor, writers rely on it, and, according to legend, a pope blessed it. Lady Astor once reportedly remarked that if she were Winston Churchill’s wife, she’d poison his coffee, to which Churchill acerbically replied: “If I were married to you, I’d drink it.”" | ||
| ]}, | ||
|
|
||
| { "title": "Two HN Announcements", | ||
| "passages": [ | ||
| "The HN community feels like it owns HN, and we like it that way. HN has become an important institution in the tech community, and though it was initially developed for YC founders it's clearly evolved into much more than that.", | ||
| "We've always felt that the best way for HN to benefit YC is simply for it to maximally benefit the community, which mostly means keeping the story and discussion quality as high as possible. We read it ourselves, so we want that as much as anyone." | ||
| ]}, | ||
|
|
||
| { "title": "SERVICE-ORIENTED ARCHITECTURE: SCALING OUR CODEBASE AS WE GROW", | ||
| "passages": [ | ||
| "Like many startups, Uber began its journey with a monolithic architecture, built for a single offering in a single city. At the time, all of Uber was our UberBLACK option and our “world” was San Francisco. Having one codebase seemed “clean” at the time, and solved our core business problems, which included connecting drivers with riders, billing, and payments. It was reasonable back then to have all of Uber’s business logic in one place. As we rapidly expanded into more cities and introduced new products, this quickly changed.", | ||
| "As core domain models grew and new features were introduced, our components became tightly coupled, and enforcing encapsulation made separation of concerns difficult. Continuous integration turned into a liability because deploying the codebase meant deploying everything at once. Our engineering team experienced rapid growth and scaling, which not only meant handling more requests but also handling a significant increase in developer activity. Adding new features, fixing bugs, and resolving technical debt all in a single repo became extremely difficult. Tribal knowledge was required before attempting to make a single change." | ||
| ]} | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <title> Homework - Note List </title> | ||
| <script src="list.js"></script> | ||
| <script src="content.js"></script> | ||
| <script src="main.js"></script> | ||
| <link rel="stylesheet" type="text/css" href="main.css" /> | ||
| </head> | ||
| <body> | ||
| <h1> Homework - Note List </h1> | ||
| <div id="note-list-wrapper"></div> | ||
| <div id="note-content-wrapper"></div> | ||
| </body> | ||
| </html> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // Karma configuration | ||
| // Generated on Mon Sep 28 2015 19:56:05 GMT+0800 (CST) | ||
|
|
||
| module.exports = function(config) { | ||
| config.set({ | ||
|
|
||
| // base path that will be used to resolve all patterns (eg. files, exclude) | ||
| basePath: '', | ||
|
|
||
|
|
||
| // frameworks to use | ||
| // available frameworks: https://npmjs.org/browse/keyword/karma-adapter | ||
| frameworks: ['mocha', 'chai', 'sinon'], | ||
|
|
||
|
|
||
| // list of files / patterns to load in the browser | ||
| files: [ | ||
| '*.js', | ||
| 'test/test-*.js' | ||
| ], | ||
|
|
||
|
|
||
| // list of files to exclude | ||
| exclude: [ | ||
| ], | ||
|
|
||
|
|
||
| // preprocess matching files before serving them to the browser | ||
| // available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor | ||
| preprocessors: { | ||
| }, | ||
|
|
||
|
|
||
| // test results reporter to use | ||
| // possible values: 'dots', 'progress' | ||
| // available reporters: https://npmjs.org/browse/keyword/karma-reporter | ||
| reporters: ['progress'], | ||
|
|
||
|
|
||
| // web server port | ||
| port: 9876, | ||
|
|
||
|
|
||
| // enable / disable colors in the output (reporters and logs) | ||
| colors: true, | ||
|
|
||
|
|
||
| // level of logging | ||
| // possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG | ||
| logLevel: config.LOG_INFO, | ||
|
|
||
|
|
||
| // enable / disable watching file and executing tests whenever any file changes | ||
| autoWatch: true, | ||
|
|
||
|
|
||
| // start these browsers | ||
| // available browser launchers: https://npmjs.org/browse/keyword/karma-launcher | ||
| browsers: ['Firefox'], | ||
|
|
||
|
|
||
| // Continuous Integration mode | ||
| // if true, Karma captures browsers, runs the tests and exits | ||
| singleRun: false | ||
| }) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| 'use strict'; | ||
|
|
||
| (function(exports) { | ||
| var ListNote = function(){ | ||
| var _listNoteContent = []; | ||
| var _wrapper = NULL; | ||
| }; | ||
|
|
||
| ListNote.prototype = { | ||
| this._wrapper = document.querySelector('#note-list-wrapper'); | ||
| start() { | ||
| this.fetchList(function(data) { | ||
| this.updateList(data); | ||
| this.drawList(); | ||
| this.preloadFirstNote(); | ||
| }); | ||
| window.addEventListener('click', function(event) { | ||
| onNoteOpen(event); | ||
| }).blind(this)); | ||
|
Contributor
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. typo.... I bet you didn't run you code before you submitted this homwork. |
||
| } | ||
|
|
||
|
|
||
| onNoteOpen(event) { | ||
| if (event.target.classList.contains('note-title')) { | ||
|
Contributor
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. If a function body is wrapped in a conditional statement with only one branch, the |
||
| var id = event.target.dataset.noteId; | ||
| var content = this._listNoteContent[id]; | ||
| window.dispatchEvent(new CustomEvent('note-open',{ detail: content })); | ||
| }; | ||
|
Contributor
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. Why you think |
||
| } | ||
|
|
||
| preloadFirstNote() { | ||
| if (this._listNoteContent.length !== 0) { | ||
| var content = this._listNoteContent[0]; | ||
| window.dispatchEvent(new CustomEvent('note-open',{ detail: content })); | ||
| } | ||
| } | ||
|
|
||
| updateList(list) { | ||
| this._listNoteContent = list; | ||
| } | ||
|
|
||
| drawList() { | ||
| var list = this._listNoteContent; | ||
| var ul = document.createElement('ul'); | ||
| ul.id = 'note-title-list'; | ||
| var buff = document.createDocumentFragment(); | ||
| list.forEach(function(note, i) { | ||
| var li = document.createElement('li'); | ||
| li.dataset.noteId = i; | ||
| li.classList.add('note-title'); | ||
| li.textContent = note.title; | ||
| // Note: buff is captured, so we now have a | ||
| // little closure naturally. | ||
| buff.appendChild(li); | ||
| }); | ||
| ul.appendChild(buff); | ||
| this._wrapper.appendChild(ul); | ||
| } | ||
|
|
||
| fetchList(afterFetch) { | ||
|
Contributor
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. WRONG: you need to Promisify this function to get the point. |
||
| var xhr = new XMLHttpRequest(); | ||
| xhr.open('GET', 'http://127.0.0.1:8000/demo-list-notes.json', true); | ||
| xhr.responseType = 'json'; | ||
| xhr.onreadystatechange = function(e) { | ||
| // Watch out: we have a mysterious unknown 'this'. | ||
| if (this.readyState === 4 && this.status === 200) { | ||
| var listData = this.response; | ||
| // The flow ends here. | ||
| afterFetch(listData); | ||
| } else if (this.status !== 200 ){ | ||
| // Ignore error in this case. | ||
| } | ||
| }; | ||
| xhr.send(); | ||
| } | ||
|
|
||
| exports.ListNote = ListNote; | ||
|
|
||
| })(window); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
|
|
||
| #note-content-wrapper { | ||
| border-top: #CCC 2px solid; | ||
| border-bottom: #CCC 2px solid; | ||
| width: 80%; | ||
| } | ||
|
|
||
| .note-title { | ||
| cursor: pointer; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| 'use strict'; | ||
| //kick off | ||
|
|
||
| document.addEventListener('DOMContentLoaded',function(event){ | ||
| var ListNote = new ListNote(); | ||
| var ContentManager = new ContentManager(); | ||
| ListNote.start(); | ||
|
Contributor
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. Serious issue: don't re-define your class with your instance. |
||
| ContentManager.start(); | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| { | ||
| "name": "map-ncu-homework-javascript", | ||
| "version": "1.0.0", | ||
| "description": "", | ||
| "dependencies": { | ||
| "karma-firefox-launcher": "^0.1.6", | ||
| "karma": "^0.13.10", | ||
| "karma-mocha": "^0.2.0" | ||
| }, | ||
| "devDependencies": { | ||
| "chai": "^3.3.0", | ||
| "karma-chai": "^0.1.0", | ||
| "karma-sinon": "^1.0.4", | ||
| "mocha": "^2.3.3", | ||
| "sinon": "^1.17.1" | ||
| }, | ||
| "scripts": { | ||
| "test": "mocha" | ||
| }, | ||
| "author": "", | ||
| "license": "ISC" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| describe('Test > ', function() { | ||
| beforeEach(function() { | ||
| subject = new ContentManager(); | ||
| }); | ||
|
|
||
| it('Test resetWrapper' function() { | ||
| // Write any pure function assertion here. | ||
| subject._wrapper = document.createElement('div'); | ||
| subject._wrapper.innerHTML = 'Test'; | ||
| subject.resetWrapper(); | ||
| assert.equal(subject._wrapper.innerHTML, '', "Failed~" | ||
|
Contributor
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. ...typo... Although you did this part correctly, and with a real method, this defect is really nasty. |
||
| }); | ||
| }); | ||
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.
Instead of binding an anonymous function, you should pass
thisas the listener of the API.