Skip to content

pushing - #7

Open
brianbixby wants to merge 1 commit into
codefellows-seattle-javascript-401d22:masterfrom
brianbixby:master
Open

brianbixby wants to merge 1 commit into
codefellows-seattle-javascript-401d22:masterfrom
brianbixby:master

Conversation

@brianbixby

Copy link
Copy Markdown

No description provided.

Comment thread lib/file-reader.js

fs.readFile(`${__dirname}/../assets/${filePath}`, function(err, data) {
if (err) return rfCallback(err);
// console.log(data.toString('hex'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corpse code wants to be deleted

Comment thread index.js
})();


// node index.js palette-bitmap.bmp transformed-palette-bitmap.bmp invert No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corpse code

Comment thread lib/file-reader.js
const readFileHelper = module.exports = (filePath, rfCallback) => {
if(filePath.indexOf('.bmp') < 0) throw new Error(`${filePath} is not a bmp file. Please visit https://online-converting.com/image/convert2bmp/ to convert this image to a bmp file.`);
if(typeof filePath !== 'string' || typeof rfCallback !== 'function') throw new Error('argument data type error');
// if(arguments.length !== 2) throw new Error('must have exactly 2 arguments');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corpse code

Comment thread lib/file-transformer.js

const transformFileHelper = module.exports = (data, transformation, tfCallback) => {
if(typeof data === 'object' && data.constructor !== Buffer || typeof data !== 'object' || typeof transformation !== 'object' && data.constructor !== Array || typeof transformation !== 'object' || typeof tfCallback !== 'function') throw new Error('argument data type error');
// if(arguments.length !== 5) throw new Error('must have exactly 3 arguments');

@KatherineHanson KatherineHanson Mar 7, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of corpse code in this file

Comment thread lib/file-writer.js
const writeFileHelper = module.exports = (data, transformedFilePath, wfCallback) => {
if(transformedFilePath.indexOf('.bmp') < 0) throw new Error(`${transformedFilePath} is not a bmp file. Please execute your command again with ${transformedFilePath}.bmp as your new file name.`);
if(typeof data === 'object' && data.constructor !== Buffer || typeof data !== 'object' || typeof transformedFilePath !== 'string' || typeof wfCallback !== 'function') throw new Error('argument data type error');
// if(arguments.length !== 5) throw new Error('must have exactly 3 arguments');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corpse code

});
});

describe('fileTransformer invertedverticalmirror', () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

invertedverticalmirror and invertedhorizontalmirror don't match what is written in the README documentation. (horizontalmirrorinverted and verticalmirrorinverted) As a result, if user tries to run transforms using only the README documentation, two of the transforms seems unable to complete.

Comment thread lib/file-transformer.js
return eval(`bmData${returnString}`);
})();
// console.log('bmdata.buffer ', bmData.buffer.toString('hex'), 'bm data.buffer ');
console.log('beginning: ', bmData.buffer.slice(1078, 11078).toString('hex'), 'beginning: ');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Output wants less console logging, to make it cleaner/easier to read

Comment thread lib/file-transformer.js

BmData.prototype.whitereplace = function() {
for(var i = 0; i <this.colorTableBuffer.length; i+=4) {
if(this.colorTableBuffer[i] == 255 && this.colorTableBuffer[i+1] == 255 && this.colorTableBuffer[i+2] == 255) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundancy can be reduced and code have be more easily understandable if redundant data is assigned to variables that can be reused

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants