{"action":"created","issue":{"url":"https://api.github.com/repos/mrbrendanwong/visualize_realize/issues/11","repository_url":"https://api.github.com/repos/mrbrendanwong/visualize_realize","labels_url":"https://api.github.com/repos/mrbrendanwong/visualize_realize/issues/11/labels{/name}","comments_url":"https://api.github.com/repos/mrbrendanwong/visualize_realize/issues/11/comments","events_url":"https://api.github.com/repos/mrbrendanwong/visualize_realize/issues/11/events","html_url":"https://github.com/mrbrendanwong/visualize_realize/issues/11","id":388399097,"node_id":"MDU6SXNzdWUzODgzOTkwOTc=","number":11,"title":"Codereviwing","user":{"login":"marquesarthur","id":1497480,"node_id":"MDQ6VXNlcjE0OTc0ODA=","avatar_url":"https://avatars1.githubusercontent.com/u/1497480?v=4","gravatar_id":"","url":"https://api.github.com/users/marquesarthur","html_url":"https://github.com/marquesarthur","followers_url":"https://api.github.com/users/marquesarthur/followers","following_url":"https://api.github.com/users/marquesarthur/following{/other_user}","gists_url":"https://api.github.com/users/marquesarthur/gists{/gist_id}","starred_url":"https://api.github.com/users/marquesarthur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/marquesarthur/subscriptions","organizations_url":"https://api.github.com/users/marquesarthur/orgs","repos_url":"https://api.github.com/users/marquesarthur/repos","events_url":"https://api.github.com/users/marquesarthur/events{/privacy}","received_events_url":"https://api.github.com/users/marquesarthur/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-12-06T21:10:19Z","updated_at":"2018-12-06T21:12:27Z","closed_at":null,"author_association":"COLLABORATOR","body":"Yet again: this does not influence your grade. It's something that I do in both 310 and 410 so we can discuss code design and learn things outside of the scope of what is being evaluated. That being said, let's roll:\r\n___\r\n**data-analyzer:**\r\n\r\n```javascript\r\nfunction analyzeBatch(commitObjects, directories, batch) {\r\n    let promises = [];\r\n    for (let i = 0; i < batchSize; i++) {\r\n        if (!directories[batch*batchSize+i]) {\r\n            break;\r\n        }\r\n        promises.push(new Promise(resolve => {\r\n            processCouplingMetric(directories[batch*batchSize+i]).then(couplingMetric => {\r\n                checkCode(directories[batch*batchSize+i]).then(styleProblems => {\r\n                    const commitIndex = path.basename(directories[batch*batchSize+i]);\r\n                    commitObjects[commitIndex].files.forEach(fileObj => {\r\n                        // save coupling metric to each file object at a commit\r\n                        const fileCouplingMetric = couplingMetric[fileObj.fileName];\r\n                        fileObj.coupling = fileCouplingMetric ? fileCouplingMetric : {};\r\n\r\n                        // save PMD style problems to each file object at a commit\r\n                        fileObj.issues = [];\r\n                        styleProblems.forEach(sp => {\r\n                            let spFileName = path.basename(sp.File);\r\n                            if (spFileName === fileObj.fileName) {\r\n                                let issue = {\r\n                                    package: sp.Package,\r\n                                    file: spFileName,\r\n                                    priority: sp.Priority,\r\n                                    line: sp.Line,\r\n                                    description: sp.Description,\r\n                                    ruleSet: sp['Rule set'],\r\n                                    rule: sp.Rule\r\n                                };\r\n                                fileObj.issues.push(issue);\r\n                            }\r\n                        });\r\n                    });\r\n                    resolve();\r\n                });\r\n            });\r\n        }));\r\n    }\r\n    return Promise.all(promises);\r\n}\r\n```\r\n\r\nToo many nested callbacks. You could declare functions and use them in your callback declarations. This will lead to more jumps (bouncing back and forth) while debugging but it can facilitate easy of readiness. As an example (that probably does not compile):\r\n\r\n```javascript\r\nfunction analyzeBatch(commitObjects, directories, batch) {\r\n    let getSytleProblems = sp => {\r\n        let spFileName = path.basename(sp.File);\r\n        if (spFileName === fileObj.fileName) {\r\n            let issue = {\r\n                package: sp.Package,\r\n                file: spFileName,\r\n                priority: sp.Priority,\r\n                line: sp.Line,\r\n                description: sp.Description,\r\n                ruleSet: sp['Rule set'],\r\n                rule: sp.Rule\r\n            };\r\n            fileObj.issues.push(issue);\r\n        }\r\n    }\r\n\r\n    let getCouplingMetrics = fileObj => {\r\n        // save coupling metric to each file object at a commit\r\n        const fileCouplingMetric = couplingMetric[fileObj.fileName];\r\n        fileObj.coupling = fileCouplingMetric ? fileCouplingMetric : {};\r\n\r\n        // save PMD style problems to each file object at a commit\r\n        fileObj.issues = [];\r\n        styleProblems.forEach(getSytleProblems);\r\n    }\r\n    let promises = [];\r\n    for (let i = 0; i < batchSize; i++) {\r\n        if (!directories[batch*batchSize+i]) {\r\n            break;\r\n        }\r\n        promises.push(new Promise(resolve => {\r\n            processCouplingMetric(directories[batch*batchSize+i]).then(couplingMetric => {\r\n                checkCode(directories[batch*batchSize+i]).then(styleProblems => {\r\n                    const commitIndex = path.basename(directories[batch*batchSize+i]);\r\n                    commitObjects[commitIndex].files.forEach(getCouplingMetrics);\r\n                    resolve();\r\n                });\r\n            });\r\n        }));\r\n    }\r\n    return Promise.all(promises);\r\n}\r\n\r\n```\r\nSame comment applies to `fileNames.forEach` at line 156\r\n\r\n**data-controller.js**\r\n\r\nAvoid silent catches. Raise exceptions or do something in case of errors. Otherwise, you have functions with inconsistent behaviors. In case of success, you return something. In case of error, you do nothing. So when you assign a variable like x = getBranches it's behavior varies.\r\n\r\n```javascript\r\n// Get the branches of a repo\r\nasync function getBranches(owner, repo) {\r\n    let result;\r\n    try {\r\n        result = await octokit.repos.getBranches({owner: owner, repo: repo});\r\n        console.log(result);\r\n    } catch (e) {\r\n        console.error(\"HttpError\", e);\r\n    }\r\n}\r\n```\r\n\r\n**data-handler-proto.js**\r\n\r\nIt's a good practice to use adapters to some 3rd party libraries and expose them through an interface (not sure how to do that in js though) but Ideally, I would wrap the rimraf thin inside an adapter-module and just return rim-raf-adapter.<some meaningful name for what you are trying to do>\r\n\r\n**data-processor-proto.js**\r\n\r\nwrap the function in the .thens inside variables similar to the discussion on **data-analyzer**. It will facilitate easy of readiness. \r\n\r\nlast but not least **dispatch.js**\r\n\r\nIf you have if else conditions for returning statements. You can work with the negation of that. It will remove one nested indentation in your code and debugging can be a little easier to follow\r\n\r\nCompare:\r\n\r\n```javascript\r\nfunction serveFile(filePath, res) {\r\n\tfs.exists(filePath, function(exists){\r\n    \tif (exists) {\r\n    \t\tfs.readFile(filePath, function(err, data){\r\n    \t\t\tif (err) {\r\n    \t\t\t\tres.statusCode = 500;\r\n    \t\t\t\tres.end(\"Could not read file.\");\r\n    \t\t\t\treturn;\r\n    \t\t\t} else {\r\n                    let ext = path.extname(filePath);\r\n\r\n\t\t    \t\tres.statusCode = 200;\r\n\t\t    \t\tres.setHeader('Content-Type', mimetype[ext] || \"text/plain\");\r\n\t\t    \t\tres.end(data);\r\n\t\t    \t\treturn;\r\n    \t\t\t}\r\n    \t\t});\r\n    \t} else {\r\n\t    \tres.statusCode = 404;\r\n\t    \tres.end(\"File not found.\");\r\n\t    }\r\n    });\r\n}\r\n```\r\n\r\nto\r\n\r\n```javascript\r\nfunction serveFile(filePath, res) {\r\n\tfs.exists(filePath, function(exists){\r\n    \tif (!exists) {\r\n\t    \tres.statusCode = 404;\r\n\t    \tres.end(\"File not found.\");\r\n                return;\r\n\t }\r\n\r\n         fs.readFile(filePath, function(err, data){\r\n    \t\t\tif (err) {\r\n    \t\t\t\tres.statusCode = 500;\r\n    \t\t\t\tres.end(\"Could not read file.\");\r\n    \t\t\t\treturn;\r\n    \t\t\t}\r\n\r\n                    let ext = path.extname(filePath);\r\n\r\n\t\t    \t\tres.statusCode = 200;\r\n\t\t    \t\tres.setHeader('Content-Type', mimetype[ext] || \"text/plain\");\r\n\t\t    \t\tres.end(data);\r\n\t\t    \t\treturn;\r\n  \r\n    \t\t});\r\n\r\n    });\r\n}\r\n```\r\n\r\nI hope some of these comments make sense. Feel free to ignore any comments that don't.\r\nAwesome project and good luck with your finals!"},"comment":{"url":"https://api.github.com/repos/mrbrendanwong/visualize_realize/issues/comments/445031206","html_url":"https://github.com/mrbrendanwong/visualize_realize/issues/11#issuecomment-445031206","issue_url":"https://api.github.com/repos/mrbrendanwong/visualize_realize/issues/11","id":445031206,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NTAzMTIwNg==","user":{"login":"marquesarthur","id":1497480,"node_id":"MDQ6VXNlcjE0OTc0ODA=","avatar_url":"https://avatars1.githubusercontent.com/u/1497480?v=4","gravatar_id":"","url":"https://api.github.com/users/marquesarthur","html_url":"https://github.com/marquesarthur","followers_url":"https://api.github.com/users/marquesarthur/followers","following_url":"https://api.github.com/users/marquesarthur/following{/other_user}","gists_url":"https://api.github.com/users/marquesarthur/gists{/gist_id}","starred_url":"https://api.github.com/users/marquesarthur/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/marquesarthur/subscriptions","organizations_url":"https://api.github.com/users/marquesarthur/orgs","repos_url":"https://api.github.com/users/marquesarthur/repos","events_url":"https://api.github.com/users/marquesarthur/events{/privacy}","received_events_url":"https://api.github.com/users/marquesarthur/received_events","type":"User","site_admin":false},"created_at":"2018-12-06T21:12:27Z","updated_at":"2018-12-06T21:12:27Z","author_association":"COLLABORATOR","body":"I'm lazy to fix my typos. Sorry :("}}
{"actor":{"display_login":"marquesarthur"}}