Skip to content

Comments

Day four#15

Open
ejdelrio wants to merge 65 commits intocodefellows-javascript-401d17:masterfrom
ejdelrio:day-four
Open

Day four#15
ejdelrio wants to merge 65 commits intocodefellows-javascript-401d17:masterfrom
ejdelrio:day-four

Conversation

@ejdelrio
Copy link

No description provided.

ejdelrio and others added 30 commits July 20, 2017 12:54
Created package.JSON file
Created directories to house files
Created transformer.js file and relative test file
Created package.JSON file
Created directories to house files
Created transformer.js file and relative test file
Removes all reds,  greens or etc...
Removes all reds,  greens or etc...
Fixed errors in the values of said object
Succesfully plugged in transformation module and completed 4 transfor…
Called all transformations within index.js
Added bitmap object and transformation requirments to index.js
Added more transformations
Refactored transformer and bitmap files. removed unneeded object cons…
@@ -0,0 +1,50 @@
'use strict';
Copy link

Choose a reason for hiding this comment

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

Good tests overall. The one thing you really aren't doing much of is testing the entirety of the write process. You should be making your you can appropriately write new bmp files and cleaning them out when you are done.

@@ -0,0 +1,45 @@
'use srict';
Copy link

Choose a reason for hiding this comment

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

Looks good

helper,
helper
);
};
Copy link

Choose a reason for hiding this comment

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

All the transforms look good.


transform.modify = (parent, blueCallback, greenCallback, redCallback) => {

const helper = (bmp, position) => (callback) => (...ind) => {
Copy link

Choose a reason for hiding this comment

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

This is a very interesting construct on how to do this so props for creativity. You may want to be careful that you aren't adding in complexity just to be complex and show off what you can do. There are definitely simpler ways to do this that will work just as well and won't add as many calls to the stack and layer this so much.

Copy link
Author

Choose a reason for hiding this comment

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

I applied the helper to clean up the callback calls in the for loop. Originally it was a lot of content per line which made readability difficult. Now that I look at it, I definitely could consolidate the helper function into a single stack. That's how it was originally.

Copy link
Author

@ejdelrio ejdelrio Jul 31, 2017

Choose a reason for hiding this comment

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

Like this right?


  const helper = (bmp, position, callback, ...ind) => {
    let hexColors = ind.map(hex => bmp.readUInt8(hex));

    return bmp.writeUInt8(callback(...hexColors), position);
  };
  let start = parent.colorTableStartPoint, end = parent.colorTableEndPoint;

  for (let i = 3 + start; i < end; i+=4) {
    helper(parent.buffer, i - 3, blueCallback, i-3, i-2, i-1);
    helper(parent.buffer, i - 2, greenCallback, i-2, i-3, i-1);
    helper(parent.buffer, i - 1, redCallback, i-1, i-2, i-3);
  }
};```

console.log(bitmapper);

for(let i = 1; i < transformations.length; i++) {
bitmapper(`./assets/${fileName}.bmp`, transform[transformations[i]], `${fileName}-${transformations[i]}`);
Copy link

Choose a reason for hiding this comment

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

This should be bitmap.renderImage() I believe as that allows it to create the files and bitmapper is just an object with properties not a function.

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.

4 participants