patternjavascriptMinor
Factoring Integers the fun way
Viewed 0 times
thewayfunfactoringintegers
Problem
Code dump:
It's mainly because I've been into functional programming lately that I tried a loopless, stateless, functional algorithm; I know that this example is absolutely facetious and that no one would do this in real life. That said, are there ways I can make this:
function factor(n) {
function count(a) {
if (a.length == 0) {
return [];
} else {
var x = a.pop();
var l = count(a);
if (l[x] == undefined) {
l[x] = 0;
}
l[x]++;
return l;
}
}
function decomp(comp, fac) {
if (comp == 1) {
return [];
}
if (fac == 1) {
return [ comp ];
}
if (comp % fac == 0) {
return decomp(fac, fac - 1).concat(decomp(comp / fac, fac));
} else {
return decomp(comp, fac - 1);
}
}
return count(decomp(n, n)).map(function(a, b) {
return [ b, a ];
}).filter(function(a, b) {
return a[1];
}).map(function(a, b) {
return "" + a[0] + "^" + a[1];
}).reduce(function(a, b) {
return a + " * " + b;
}).replace("^1", "");
}It's mainly because I've been into functional programming lately that I tried a loopless, stateless, functional algorithm; I know that this example is absolutely facetious and that no one would do this in real life. That said, are there ways I can make this:
- More efficient (e.g. less data),
- More concise (which keeping the cool functional aspect), or
- More functional (in a good way) in paradigm?
Solution
-
Keystrokes are (almost) free. It's not like
-
Your
(The logic can be reduced to
-
It's worth mentioning that comparisons against
-
You've got a redundant
-
You can use
or alternatively, chained conditional operators (commonly called ternary operators):
-
-
You're replacing only the first instance of
-
-
-
You're using a lot of functions at the end there.
Here's my rendition. It's a little bit less elegant with the return... oh well. I'm using a utility function
Keystrokes are (almost) free. It's not like
decomp looks any better than decompose. (Moving forward, I'm going to assume you made the rename.)-
Your
count() function is cute. For extra fun, you could replace it with Array.reduce() (which you may know as foldl):decompose(n, n).reduce(function(accumulator, value) {
if (typeof accumulator[value] === "undefined") {
accumulator[value] = 0;
}
accumulator[value]++;
return accumulator;
}, []).map ...(The logic can be reduced to
prev[cur]++ || (prev[cur] = 1) or prev[cur] = prev[cur] + 1 || 1 if desired.)-
It's worth mentioning that comparisons against
undefined are typically done with ===.-
You've got a redundant
else in decompose().-
You can use
switch(true) syntactic sugar to make decompose() prettier:function decompose(composite, factor) {
switch(true) {
case composite == 1:
return [];
case factor == 1:
return [composite];
case composite % factor == 0:
return decompose(factor, factor - 1).concat(decompose(composite / factor, factor));
default:
return decompose(composite, factor - 1);
}
}or alternatively, chained conditional operators (commonly called ternary operators):
function decompose(composite, factor) {
return composite == 1 ? [] :
factor == 1 ? [composite] :
composite % factor != 0 ? decompose(composite, factor - 1) :
decompose(factor, factor - 1).concat(decompose(composite / factor, factor));
}
}-
decompose(n, n) is ugly. Maybe you rewrite the contract of decompose to int (maybe int) -> array?-
You're replacing only the first instance of
^1. You'll want to do .replace(/\^1/g, "") instead.-
factor(1) doesn't return 1 as expected. You can fix this by switching your first two ifs.-
factor(0) recurs infinitely.-
You're using a lot of functions at the end there.
Here's my rendition. It's a little bit less elegant with the return... oh well. I'm using a utility function
when to avoid potentially ugly ternary nesting. The double reduce bugs me but I can't think of a better way right now.function factor(n) {
function when(condition, string) {
return condition ? string : "";
}
function decompose(composite, factor) {
return factor === undefined ? decompose(composite, composite) :
factor 1, "^ " + value));
}, "").substr(3);
}Code Snippets
decompose(n, n).reduce(function(accumulator, value) {
if (typeof accumulator[value] === "undefined") {
accumulator[value] = 0;
}
accumulator[value]++;
return accumulator;
}, []).map ...function decompose(composite, factor) {
switch(true) {
case composite == 1:
return [];
case factor == 1:
return [composite];
case composite % factor == 0:
return decompose(factor, factor - 1).concat(decompose(composite / factor, factor));
default:
return decompose(composite, factor - 1);
}
}function decompose(composite, factor) {
return composite == 1 ? [] :
factor == 1 ? [composite] :
composite % factor != 0 ? decompose(composite, factor - 1) :
decompose(factor, factor - 1).concat(decompose(composite / factor, factor));
}
}function factor(n) {
function when(condition, string) {
return condition ? string : "";
}
function decompose(composite, factor) {
return factor === undefined ? decompose(composite, composite) :
factor <= 1 ? [composite] :
composite <= 1 ? [] :
composite % factor != 0 ? decompose(composite, factor - 1) :
decompose(factor, factor - 1).concat(decompose(composite / factor, factor));
}
return decompose(n).reduce(function(accumulator, value) {
accumulator[value] = accumulator[value] + 1 || 1;
return accumulator;
}, []).reduce(function(accumulator, value, index) {
return accumulator + when(value, " * " + index + when(value > 1, "^ " + value));
}, "").substr(3);
}Context
StackExchange Code Review Q#59432, answer score: 4
Revisions (0)
No revisions yet.