Skip to content

Instantly share code, notes, and snippets.

@edas
Last active August 14, 2016 10:05
Show Gist options
  • Save edas/8c1f5b351732d499d8c7d3c3c1d80435 to your computer and use it in GitHub Desktop.
Save edas/8c1f5b351732d499d8c7d3c3c1d80435 to your computer and use it in GitHub Desktop.
import program from "commander";
import glob from "glob";
import path from "path";
import jsonfile from "jsonfile";
import sizeOf from "image-size";
program
.version("1.0.0")
.arguments('<manifest> <dir>')
.action(function(manifest, dir) {
const source_path = path.join(dir, "**/*.jpg");
const options = { };
glob(source_path, options, function (err, files) {
if (err) {
console.error("ERROR: %s", err.message);
} else {
let flag = file.length;
let filesMeta = [ ];
files.map(function (file) {
sizeOf(file, function(err, dimensions)) {
if (err) {
console.error("ERROR: %s", err.message);
} else {
dimensions.path = file;
dimensions.name = path.basename(file);
filesMeta.push(dimensions);
flag--;
if (flag===0) {
jsonfile.writeFile(manifest, filesMeta, function (err) {
if (err) {
console.error("ERROR: %s", err.message);
} else {
console.info("SUCCESS!");
}
});
}
}
});
});
}
});
}).parse(process.argv);
if (process.argv.length < 3) {
program.outputHelp();
exit(1);
}
import program from "commander";
import glob from "glob";
import path from "path";
import jsonfile from "jsonfile";
import sizeOf from "image-size";
program
.version("1.0.0")
.arguments('<manifest> <dir>')
.action(writeFilesDimensions)
.parse(process.argv);
function writeFilesDimensions(manifest, dir) {
const source_path = path.join(dir, "**/*.jpg");
const options = { };
const next_callback = function (err, files) {
if (err) {
console.error("ERROR: %s", err.message);
} else {
getFilesDimensions(files, manifest);
}
};
glob(source_path, options, next_callback);
}
function getFilesDimensions(files, manifest) {
let flag = file.length;
let filesMeta = [ ];
files.map(function (file) {
sizeOf(file, function(err, dimensions)) {
if (err) {
console.error("ERROR: %s", err.message);
} else {
dimensions.path = file;
dimensions.name = path.basename(file);
filesMeta.push(dimensions);
flag--;
if (flag===0) {
writeFile(filesMeta, manifest);
}
}
});
});
}
function writeFile(filesMeta, manifest) {
jsonfile.writeFile(manifest, filesMeta, success);
}
function success(err) {
if (err) {
console.error("ERROR: %s", err.message);
} else {
console.info("SUCCESS!");
}
}
if (process.argv.length < 3) {
program.outputHelp();
exit(1);
}
import program from "commander";
import glob from "glob";
import path from "path";
import jsonfile from "jsonfile";
import sizeOf from "image-size";
program
.version("1.0.0")
.arguments('<manifest> <dir>')
.action(writeFilesDimensions)
.parse(process.argv);
function writeFilesDimensions(manifest, dir) {
let dest_file = manifest;
let counter = 0;
let files_meta = [ ];
const error = function (err) {
console.error("ERROR: %s", err.message);
}
const cb = function(next_cb) {
return function (err, data) {
if (err) {
error(err);
} else {
next_cb(data);
}
};
};
const success = function () {
console.info("SUCCESS!");
}
const writeManifest = function() {
jsonfile.writeFile(dest_file, files_meta, success);
};
const finishAllFilesMeta = function () {
counter--;
if (counter === 0) {
writeManifest();
}
};
const addFileMeta = function (file, dimensions) {
dimensions.path = file;
dimensions.name = path.basename(file);
files_meta.push(dimensions);
finishAllFilesMeta();
};
const getFileDimension = function (file) {
const next_cb = function (dimensions) { addFileMeta(file, dimensions); };
sizeOf(file, cb(next_cb) );
};
const getFilesDimensions = function(files) {
counter = files.length;
files.map( getFileDimension ) ;
};
const getFiles = function(dir) {
};
getFiles(dir);
}
if (process.argv.length < 3) {
program.outputHelp();
exit(1);
}
import program from "commander";
import glob from "glob";
import path from "path";
import jsonfile from "jsonfile";
import sizeOf from "image-size";
import promisify from "promisify-node";
const glob_p = promisify(glob, undefined, true);
const write_p = promisify(jsonfile.writeFile, undefined, true);
const size_p = promisify(imageSize, undefined, true);
program
.version("1.0.0")
.arguments('<manifest> <dir>')
.action(writeFilesDimensions)
.parse(process.argv);
function writeFilesDimensions(manifest, dir) {
const source_path = path.join(dir, "**/*.jpg");
const options = { };
glob_p(source_path, options)
.then( getFilesDimensions )
.then( function (data) { writeManifest(manifest, data); } )
.then( success )
.catch( error );
}
function error(err) {
console.error("ERROR: %s", err.message);
}
function sucess() {
console.info("SUCCESS!");
}
function writeManifest(manifest, data) {
return write_p(manifest, data);
}
function getFilesDimensions(files) {
const promises = files.map( getFileDimension ) ;
return Promise.all( promises );
}
function getFileDimension(file) {
return size_p(file).then(function (dimensions) {
dimensions.path = file;
dimensions.name = path.basename(file);
return dimensions;
});
}
if (process.argv.length < 3) {
program.outputHelp();
exit(1);
}
import program from "commander";
import glob from "glob";
import path from "path";
import jsonfile from "jsonfile";
import sizeOf from "image-size";
import promisify from "promisify-node";
const glob_p = promisify(glob, undefined, true);
const write_p = promisify(jsonfile.writeFile, undefined, true);
const size_p = promisify(imageSize, undefined, true);
program
.version("1.0.0")
.arguments('<manifest> <dir>')
.action(writeFilesDimensions)
.parse(process.argv);
async function writeFilesDimensions(manifest, dir) {
try {
const source_path = path.join(dir, "**/*.jpg");
const options = { };
const files = await glob_p(source_path, options);
let files_meta = [ ];
for (const file of files) {
let dimensions = await size_p(file);
dimensions.path = file;
dimensions.name = path.basename(file);
files_meta.push(dimensions);
}
await write_p(manifest, filesMeta);
console.info("SUCCESS!");
} catch(err) {
console.error("ERROR: %s", err.message);
}
}
if (process.argv.length < 3) {
program.outputHelp();
exit(1);
}
@borisschapira
Copy link

borisschapira commented Aug 12, 2016

Pour la "ligne 24", tu peux peut-être utiliser le Currying, une pratique de programmation fonctionnelle qui te permet de transformer une fonction prenant plusieurs paramètres en un ensemble de fonctions préparées. Tu aurais ainsi à disposition une fonction writeManifestBis qui ne prendrait que data comme param.

@rik
Copy link

rik commented Aug 12, 2016

En complément du commentaire

async function write_file_dimensions(manifest, dir) {
    const source_path = path.join(dir, "**/*.jpg");
    const options = { };

    try {
        filepaths = await glob_p(source_path, options);
        file_metadatas = await Promise.all(filepaths.map(get_metadata))
        write_p(manifest, file_metadatas)
    } catch(err) {
        console.error(`ERROR: ${err.message}`);
    }
}

async function get_metadata(filepath) {
    const dimensions = await size_p(filepath);
    return {
        ...dimensions,
        path: filepath,
        name: path.basename(filepath),
    }
}

@n1k0
Copy link

n1k0 commented Aug 12, 2016

Il manque sans doute une solution à base de générateurs, qui ressemblerait fortement à celle basée sur async/await; la nuance principale résiderait dans l'écriture des tests, où les valeurs récupérées par yield à chaque itération se mockent plus facilement.

@edas
Copy link
Author

edas commented Aug 12, 2016

@DavidBruant: Corrigé pour les valeurs de retour, merci. Je suis trop habitué aux facilités de Ruby.
Effectivement aussi pour le côté séquentiel de la boucle par rapport au parallélisme de Promise.all

@borisschapira: Oui, j'ai hésité pour la fonction qui génère une fonction, d'autant que j'avais commencé un pas dans cette direction avec cb sur la version closures. Maintenant j'ai trouvé que c'était déjà assez complexe à suivre ainsi. Une closure m'apparaissait finalement aussi simple si c'est pour l'utiliser une seule fois.

@rik: C'est vrai que ta solution pour retourner l'objet contenant nom de fichier et dimensions est vachement plus classe que la mienne. Modifier l'objet dimension directement n'était pas super élégant.

@Lythom
Copy link

Lythom commented Aug 14, 2016

Avant toute chose, merci pour ce travail de "mise au propre" de tes recherches en JavaScript. J'ai découvert des choses intéressantes (pratiques de CLI en JS notamment) ! J'espère que je vais pouvoir à mon tour t'apporter quelques éléments.

De mon côté j'aurai pris l'approche Promise sans hésiter. J'ai repris ta version et apporté quelques modifications :
https://gist.github.com/Lythom/d49e1fa67d2a4cb14b79e42554b05c68#file-4-promise-js

Liste des changements :

  • Suppression de promisify et utilisation directe de l'objet Promise.
    • Le code de la lib est difficile à lire et plutôt complexe : en cas de problème le debug est plus facile sans passer par la lib
    • L'utilisation directe et explicite des Promise est facile à lire et à comprendre, là où le fonctionnement de la fonction de haut niveau nécessite de se documenter sur la librairie. (réduire le nombre d'indirections facilite la lecture du code de manière générale)
  • Pour glob, utilisation directe du callback plutôt que de passe par une promise.
    • Sans promisify encapsuler le tout sous une Promise juste pour gérer le flux d'erreur est plus complexe que d'avoir une ligne pour le cas d'erreur
  • Utilisation des arrow fonctions es6 dans la chaine de "then"
    • Question de préférence, mais je trouve qu'expliciter à chaque ligne l'entrée utilisée par le traitement permet d'appréhender plus facilement l'ensemble du traitement. A chaque ligne, la variable (à nommer explicitement !) est à la fois l'output de la fonction du dessus et l'input de la fonction adjacente.
  • Inlining de la fonction getFilesDimensions.
    • La fonction fait peu de chose et ça permet d'expliciter l'utilisation de la Promise comme entrée de la chaîne de traitement.
  • Utilisation de require à la place de import
    • Et on peut alors se passer de babel complètement =)
  • Je n'ai pas ajouté de commentaires de code pour pouvoir comparer la lisibilité du code seul, mais en vrai sur un livrable, j'aurai mis quelques indications sur les lignes les moins évidentes pour synthétiser l'action réalisée.

Autres commentaires sur les discussions :

  • L'utilisation de la closure pour faire descendre le manifest en paramètre est une bonne approche car les 2 variables utilisées par la fonctions sont ainsi faciles à tracer : on ajoute pas d'indirection. Avec la chaîne de traitement en style "arrow function" ça n'est d'ailleurs plus choquant du tout.
  • Je n'ai pas l'habitude d'utiliser async/await, pour le moment j'évite pour des raisons de debug : le code est nécessairement transpilé et je sais pas si j'ai très envie d'avoir à mettre le nez dedans.
  • D'une manière générale les appels asynchrones ne sont pas si fréquents (pour le web en tout cas, évidemment ça dépend toujours de ce que fait l'application), et ceux qui sont fréquents (fetch d'API, d'urls) retournent des Promise directement.

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