Skip to content
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

Transform Function #68

Merged
merged 5 commits into from Jul 31, 2014
Merged

Transform Function #68

merged 5 commits into from Jul 31, 2014

Conversation

@arb
Copy link
Contributor

arb commented Jul 25, 2014

Description

Adds a transform function that lets you pass in a source object and a mapping object. This lets you take one object and transform it into a new schema more suited to the current need.

// {
// person: {
// address: {
// lineOne: '123 main street',

This comment has been minimized.

Copy link
@danielb2

danielb2 Jul 25, 2014

Contributor

source has address.one and object composing has person.address.lineOne while the result only has the latter.
Probably not intended.

This is very close to applyToDefaults but the short-hand is interesting.

This comment has been minimized.

Copy link
@arb

arb Jul 25, 2014

Author Contributor

This seems right to me... I took it right from the very first passing unit test.

This comment has been minimized.

Copy link
@danielb2

danielb2 Jul 25, 2014

Contributor

yepp, it's good. nevermind.

@danielb2

This comment has been minimized.

Copy link
Contributor

danielb2 commented Jul 28, 2014

OK. I know what threw me off: how about renaming this to "transform" instead? You even use it to describe what the function does

@arb

This comment has been minimized.

Copy link
Contributor Author

arb commented Jul 28, 2014

Renamed.

@arb arb changed the title Compose Function Transform Function Jul 28, 2014

exports.transform = function (source, transform, options) {

var result = {};

This comment has been minimized.

Copy link
@geek

geek Jul 30, 2014

Member

Needs an assert of the source and transform (target?)

exports.assert(source === null || source === undefined || typeof source === 'object', 'Invalid source value: must be null, undefined, or an object');

Return early if its null or undefined...


var result = {};
var keys = Object.keys(transform);
var reach = exports.reach;

This comment has been minimized.

Copy link
@geek

geek Jul 30, 2014

Member

not a common style/practice to shortcut to the exports function... probably best for now to be consistent and explicit, refer to reach through exports.reach, same for assert.

});

it('uses the reach options passed into it', function (done) {
var result = Hoek.transform(source, {

This comment has been minimized.

Copy link
@geek

geek Jul 30, 2014

Member

newline

});

it('works to create shallow objects', function (done) {
var result = Hoek.transform(source, {

This comment has been minimized.

Copy link
@geek

geek Jul 30, 2014

Member

newline

@@ -1347,4 +1347,95 @@ describe('Hoek', function () {
done();
});
});

describe('#transform', function () {

This comment has been minimized.

Copy link
@geek

geek Jul 30, 2014

Member

Please add a test for a source that has null values as well as non-string child values.

@arb

This comment has been minimized.

Copy link
Contributor Author

arb commented Jul 30, 2014

@wpreul should be good to go now.

});

it('throws an error on invalid arguments', function (done) {
expect(function() {

This comment has been minimized.

Copy link
@geek

geek Jul 31, 2014

Member

newline


it('throws an error on invalid arguments', function (done) {
expect(function() {
var result = Hoek.transform(NaN, {});

This comment has been minimized.

Copy link
@geek

geek Jul 31, 2014

Member

newline

lineTwo: 'PO Box 1234',
title: 'Warehouse',
region: 'CA',
provice: null

This comment has been minimized.

Copy link
@geek

geek Jul 31, 2014

Member

typo: province


it('uses the reach options passed into it', function (done) {

var result = Hoek.transform(source, {

This comment has been minimized.

Copy link
@geek

geek Jul 31, 2014

Member

This will probably read more clearly to declare the schema and options as local vars and pass them into the transform function


it('throws an error on invalid arguments', function (done) {

expect(function() {

This comment has been minimized.

Copy link
@geek

geek Jul 31, 2014

Member

function () {

geek added a commit that referenced this pull request Jul 31, 2014
Transform Function
@geek geek merged commit 165bb82 into hapijs:master Jul 31, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@hueniverse hueniverse added this to the 2.4.0 milestone Aug 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.