0

I'm using Promises to synchronize my hashing functions within my node application:

var example_int = '1000121';

function MakeRequest(value){
    var hash_result = RequestHash(example_int);
    hash_result.then(function(v){
      //Do stuff with v...
    })
}

function RequestHash(value){
    return(Hash(value))
}

function Hash(value){
    var HashPromise = new Promise(function(resolve, reject){
        var crypto = require('crypto');
        var hash = crypto.createHash('md5');

        hash.on('readable', () => {
            var data = hash.read();
            if(data){
                var output = data.toString('hex');
                resolve(output);
            }
        })
        hash.write(value);
        hash.end();               
    })
    return(HashPromise);
}

MakeRequest();

However, I want to be able to access the resulting hash values from outside the Promise functions. My initial approach was to use re-assignment:

var example_int = '1000121';

function MakeRequest(value){
    var hash_result = RequestHash(example_int);
    //Hash output assigned to example_int....
}

function RequestHash(value){
    return(Hash(value))
}

function Hash(value){
    var HashPromise = new Promise(function(resolve, reject){
        var crypto = require('crypto');
        var hash = crypto.createHash('md5');

        hash.on('readable', () => {
            var data = hash.read();
            if(data){
                var output = data.toString('hex');
                example_int = output; //Re-assignment...
                resolve(output);
            }
        })
        hash.write(value);
        hash.end();               
    })
    return(HashPromise);
}

MakeRequest();

Is there a safer way and/or more efficient method of performing this without the reliance on re-assingment?

Babra Cunningham
  • 2,949
  • 1
  • 23
  • 50
  • 1
    There is a canonical thread that covers "returning" values from asynchronous code: http://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call – Tomalak Nov 23 '16 at 18:00
  • 1
    If it's async, you can't access the value outside the async callback. That happens at some indeterminate time in the future so the only way you know the timing is to use the value inside the callback (or in some function you call from inside the callback). That's just how async programming works. – jfriend00 Nov 23 '16 at 18:23
  • You seem to have mis-copied an example from the node.js crypto documentation. The hash itself is NOT a stream and is not async. In the example in the node.js documentation, they create an inputStream and are computing a running hash of the contents of that asynchronous stream. The hash compute operation itself is NOT async. – jfriend00 Nov 23 '16 at 18:44
  • No, you cannot immediately assign the result (which you get asynchronously) to the variable. But you can assign the promise for it - just `return` it all the way through your functions. Where do you actually need to use the `example_int`, anywhere except `MakeRequest`? – Bergi Nov 23 '16 at 18:54

2 Answers2

3

The hash API is not asynchronous in node.js, so there is no need for promises to begin with.

Code as simple as this:

var crypto = require('crypto');

var exampleValue = '1000121';
var md5Val = crypto.createHash('md5').update(exampleValue).digest('hex');
// -> '4f0b7e8fcb0cfcfce1c77e39864e7ca4'

does the job just fine. If you want, wrap it in a function.

If your real issue involves an asynchronous part then you need to ask the question with more details.

Tomalak
  • 332,285
  • 67
  • 532
  • 628
  • This appears to be the correct approach. There's nothing async here so no reason to use promises at all. – jfriend00 Nov 23 '16 at 19:17
2

I've been reading a lot from Eric Elliot lately. One of the paradigms he highly suggests is to implement "pure functions" (meaning functions which do not rely upon a shared mutable state, such as your example_int re-assignment approach).

I agree with @Tomalak on the synchronous nature of the Hash API and his suggestion (but using your provided code).

I wanted to take a stab at answering your question in terms of implementation. This may not be perfect, but I wanted to try answering this (any feedback from others is welcome). Modularize this code for multiple configurations, and have the generate/MakeRequest method, return a promise. Then you can do as you need from your consuming app code (this also simplifies your testing approach since you are not checking on shared state value, but instead...just I/O.

// hasher.js
const crypto = require('crypto');
const Hasher = module.exports = function Hasher(algorithm = 'md5',digest = 'hex', ...opts) {
    this.digest = options.digest || 'hex';
    this.algorithm = options.algorithm || 'md5';
    // For other hashing algorithms, spread `opts` and assign to instance properties 
};

// Will need improvements or abstraction to support other hashing algorithms
// The below code is synchronous, and does not "need" to use Promises, but is being used to try and answer the original question.
Hasher.prototype.generate = (data) => {
    return new Promise((resolve, reject) => {
        if(!data) reject(new Error('Hasher.generate() requires `data` parameter'));
        resolve(crypto([this.algorithm]).update(data).digest(this.digest));
    }); 
};

// app.js
const Hasher = require('./hasher.js'); // Create new hashers as needed

const md5Hasher = new Hasher(); // defaults to MD5 w/ hex digest
let someData = 12345;
md5Hasher(someData)
.then((hashedData) => {
    someData = hashedData;
})
.catch(e) {
    console.error(e);
    throw e;
};
Benjamin Dean
  • 1,218
  • 11
  • 11
  • You don't need `try` blocks in the synchronous part of `new Promise` callbacks – Bergi Nov 23 '16 at 18:56
  • Why don't I need that please? What if there is an operational error with the crypto library, doesn't implementing the try..catch make sure that doesn't get lost? – Benjamin Dean Nov 23 '16 at 19:03
  • Also you cannot use an arrow function as a constructor. Either use a standard `function` expression, or a `class` declaration. – Bergi Nov 23 '16 at 19:05
  • Thanks, I am still learning ES6, and I did not know that about arrow functions with constructors, I will update that now. – Benjamin Dean Nov 23 '16 at 19:07
  • Why are you using a promise for a synchronous operation in `.generate()`? Why is that a desirable thing to do? – jfriend00 Nov 23 '16 at 19:13
  • @Bergi I think I have made the necessary adjustments to my suggested answer as you requested. Does this look more appropriate? – Benjamin Dean Nov 23 '16 at 19:13
  • @jfriend00 I just adjusted that based on previous comment. – Benjamin Dean Nov 23 '16 at 19:14
  • My comment still stands. There do not appear to be ANY asynchronous operations in this hash code so I do not see why promises are being used at all. Taking a synchronous operation and making it asynchronous with promises just complicates things and is rarely ever desirable. – jfriend00 Nov 23 '16 at 19:15
  • 1
    Ah, I see what you are saying now. You're saying, "If the function isn't doing asynchronous work, a Promise is heavy-handed.". Meaning that the Promise could be ripped out entirely. Yes, you're correct in that regards, but I was trying to demonstrate one way to update a value outside of a promise structure (which is semi-related) to the code initially provided in the question. The additional module is not really necessary, but I thought it helped demonstrate the answer to the original question. – Benjamin Dean Nov 23 '16 at 19:20