1

I have these three functions that basically do the same thing. They take data coming from the promise and make an object with the same properties for each function.

I would like to DRY this out:

var getFiles = function() {
    return wg.getFiles().then(function(data) {
        processData(data, {
            type: "File",
            title: x.filename,
            created: x.Created,
            path: x.path
        })
    })
}

var getEvents = function() {
    return wg.getEvents().then(function(data) {
        processData(data, {
            type: "Event",
            title: x.Title,
            created: x.Created,
            path: x.path
        })
    })
}

var getFeedback = function() {
    return wg.getFeedback().then(function(data) {
        processData(data, {
            type: "Review",
            title: "by " + x.Author,
            created: x.Created,
            path: x.path
        })
    })
}
var processData = function(data, props) {
    var x = _(data)
    .map(function(x) {return props})
    .value()
.map(function(x) {
    activties.push(x)
})
}

I would like to DRY this out by changing the processData function to something like this:

var processData = function(data, props) {
    var x = _(data)
    .map(function(x) {
        return {
            type: x[props[0]],
            title: x[props[1]],
            created: x[props[3]],
            path: "/" + x[props[4]]
        }
    })
    .value()
.map(function(x) {
    activties.push(x)
})
}

Then I could call it like this:

var getFiles = function() {
    return wg.getFiles().then(function(data) {
        processData(data, ['filename', 'created', ['FileRef']['lookupValue']])

    })
}

That's what I have in mind but if anyone has anything better I'm open.

Batman
  • 5,563
  • 18
  • 79
  • 155
  • 2
    Out of curiosity, do you think there's something wrong with your current approach? Sure, the `processData()` method looks a bit unsightly, but for the sake of making everything else readable I'd say it's fine. – Reinstate Monica Cellio Oct 20 '16 at 13:56
  • You could invert the order of parameters in your processData fn. From function (data, props) to function (props, data) and then benefit from partial application if you want. – Delapouite Oct 20 '16 at 13:58
  • @Archer I wouldn't say I think there's something wrong but I do like keeping my code DRY if it's possible or simple enough to accomplish. If trying to DRY this out results in an extra 20 lines of code, then yea it's not worth the effort. But if it's simple enough to do, I'd like to learn how. – Batman Oct 20 '16 at 14:00
  • 1
    Just to note I voted to close as this is primarily opinion-based, I'd say what you have is fine. – Fraser Oct 20 '16 at 14:01
  • Just curious, what is `x` and what is `activities`? And why don't you `return` the result of `processData(…)` from the promise? It doesn't look like your current code is working. – Bergi Oct 20 '16 at 14:04
  • @Fraser that's fair, thanks for the comment. – Batman Oct 20 '16 at 14:05
  • Are the differences `created` vs `Created` and `x.FileRef.lookupValue.split("_")[0]` vs `x.FileRef.lookupValue` intentional? – Bergi Oct 20 '16 at 14:06
  • @Bergi, activities is an array that contains the result. I plan on using it to display like a latest items on a site. So I'm pushing Events, Files, Reviews to that 1 array and I'll handle that later when all 3 promises are done. `X` is just a generic variable since `processData` is a utility function. – Batman Oct 20 '16 at 14:07
  • @Batman I meant the `x` in the `get…` functions where it is not declared, not the `x` in the callbacks in `processData`. – Bergi Oct 20 '16 at 14:09
  • I'd agree with @Fraser - this is fine. Go with it until you have a need to change it, if ever. – Reinstate Monica Cellio Oct 20 '16 at 14:10
  • @Bergi, not really. Cleaned in it up the `wg` object. – Batman Oct 20 '16 at 14:14
  • DRY is often overused. Theres no problem with repeating **some** code if it aids readability. – Liam Oct 20 '16 at 14:21

1 Answers1

1

Don't pass property names, pass functions:

function getFiles() {
    return wg.getFiles().then(processData("File", _.property("filename")));
}

function getEvents() {
    return wg.getEvents().then(processData("Event", _.property("Title")));
}

function getFeedback() {
    return wg.getFeedback().then(processData("Review", function(x) { return "by " + x.Author; })));
}

function processData(type, makeTitle) {
    return function(data) {
        return data.map(function(x) {
            return {
                type: type,
                title: makeTitle(x),
                created: x.Created,
                path: x.path
            };
        });
    };
}

Promise.all([getFiles(), getEvents(), getFeedback()])
.then(function(arrays) {
    var activities = _.flatten(arrays);
    …
});

For convenience I used some underscore/lodash functions, but you can do it without them as well.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • I noticed that by cleaning up the properties coming back from the `wg` object and making them consistent, it makes the whole problem simpler. – Batman Oct 20 '16 at 14:25
  • 1
    Yes, but the approach to solving the problem stays the same :-) If they weren't cleaned up, you'd just have to use one callback function per difference. – Bergi Oct 20 '16 at 14:27
  • I don't really understand the `_.prop()` and `makeTitle(x)` and how they work today. I'm trying to find documentation for it but I'm not seeing anything. – Batman Oct 20 '16 at 14:29
  • 1
    @Batman Ooops, I confused it with [`R.prop`](http://ramdajs.com/docs/#prop), I meant [`_.property`](http://underscorejs.org/#property). `makeTitle` is just the function that was passed as an argument, like the `function (x) { return … }` expression in `getFeedback` – Bergi Oct 20 '16 at 14:38
  • I 'm trying to implement your solution but I'm working on IE8 so `Promise.all` won't work here. I'm trying jQuery `$.when.apply($, [getFiles(), getEvents(), getFeedback()])` but it's only return the results from the first array. Any ideas? – Batman Oct 20 '16 at 15:25
  • 1
    @Batman `$.when` doesn't create an `arrays` array but [passes multiple `arguments`](http://stackoverflow.com/a/19090635/1048572). So either use `_.flatten(arguments)` or just write it out to `$.when(getFiles(), getEvents(), getFeedback()).then(function(files, events, feedbacks) { var activities = files.concat(events, feedbacks); })` – Bergi Oct 20 '16 at 15:27
  • Nvm the second answer here solved it. Thanks! http://stackoverflow.com/questions/4878887/how-do-you-work-with-an-array-of-jquery-deferreds – Batman Oct 20 '16 at 15:32