This code is supposed to count each character in a given string and account for error handling and type casting as well, and it should do it in the most efficient manner possible, with O(n) being an upper limit for both time and memory. I have gone through my process for problem solving which involves explaining the problem in my own words, looking at the inputs, the outputs, if the answer is even possible given the I/O, and then refactoring my code. Please, if anyone sees a better way to accomplish this then let me know.
function characterCounter(str) {
// Error Handling
// No input
if (!str) {
return "String input required";
}
// attempt to type cast non strings
if (typeof str !== "string") {
str = str.toString();
// input could not be successfully type casted
if (typeof str !== "string") {
return "Invalid input";
}
}
const stringLen = str.length;
const result = {};
let i = 0;
// loop through each character for O(n) time complexity and O(n) space complexity
// since the object grows based on input size
for (; i < stringLen; i++) {
// If string not in object as a key then add it and set value to 1
if (result[str[i]] === undefined && str[i] !== " ") {
result[str[i].toLowerCase()] = 1;
} else {
if (str[i] !== " ") {
// add one to value at current key
result[str[i]]++;
}
}
}
return result;
}
// Returns an object which allows O(1) for insert, deletion, access, and O(n) for searching...
console.log(characterCounter("Hello, World!"));
2 Answers 2
Some minor problems
The function counts characters in a string and is called
characterCounter
yet that is not what it does.It checks if the argument is not falsy like and can be coerced to a string (which is already known to be true) and if so counts the number of lowercase characters, not spaces, and returns an object containing counts.
This is very problematic as any code that calls the function must also check that the result it returns is not an
Object
containing character counts but a string telling the caller that he does not know what a string is. (rude)It is a semantic grinch that I would never use in my code as its name would make meaningless gibberish out of my perfect code (Yes all coders write perfect code, me more so :P).
Maybe
mapCharacterCounts
would restore the balance.Object.toString
is inherited fromObject
, any argument passed to the function will have atoString
function, thus the return"Invalid input"
will only happen if for some reasontoString()
returned a non stringYes possible if someone changes the function but that is not your function's problem.
If we imagine that a argument could not be converted to a string then there would not be a
toString
function would there?The function incorrectly counts the number of characters in
""
gets ("String input required"
... wtf""
is a string :( me confuseded ),0
gets ("String input required"
)"0"
gets ({"0":1}
),true
gets ("String input required"
),false
gets ({f:1,a:1,l:1,s:1,e:1}
)
to name a few.
If I pass something that is not string like and that would be coerced to a non representative string, eg
{}
is[object Object]
that would return a Object that counts a very knowable quantity that does not represent the question asked of the function.Personally if I called the function
characterCounter({})
I would expect an empty object as the return.Hang on now I am confused, maybe I should only call the functions with something I know to be string like?
Using objects as maps is very ECMAScript5 and is hard to use. It would be much more useful if it returned a
Map
of character countsYou don't count spaces, but you count tabs, form feed, returns and all the other white space characters.
Why is a space uncountable yet a tab can count (in my book tab is always 4 spaces)
What is a string?
So to make the function practical we first remove the problem of defining what is a string. Its too complicated and can change depending on need.
If you want you can create additional functions to do the vetting and coercion that callers can use to help them work out what type their variables are!!
The function has one role and that is Map Character Counts.
Maybe it would be more practical as?
// Function count characters in the str returning a map by character
// as {character, count}
// It's the caller's responsibility to ensure the correct type
function mapCharacterCounts(str) {
str = (str + "").replace(/\s/g,"").toLowerCase();
const result = new Map();
var i;
for (i = 0; i < str.length; i++) {
const c = str[i];
if (result.has(c)) { result.get(c).count ++ }
else { result.set(c, {character : c, count : 1}) }
}
return result;
}
-
\$\begingroup\$ Thanks for the detailed answer. Using a Map didn't cross my mind but that makes 100% sense now that I see it! Thanks again. \$\endgroup\$Brian Ruff– Brian Ruff2018年12月04日 04:53:50 +00:00Commented Dec 4, 2018 at 4:53
The algorithm is not working quite as you may expect:
if (result[str[i]] === undefined && str[i] !== " ") { result[str[i].toLowerCase()] = 1; } else { if (str[i] !== " ") { // add one to value at current key result[str[i]]++; } }
If you enter "Hello, World! - Hi!"
the two upper case 'H' are only counted as one, because they both evaluate to true
in the first if-statement - because you test for an upper case 'H' but save a lower case 'h'.
In the same place you filter spaces out both in true
and false
, so you could do that as the first thing to do in the loop.
All in all that could be like:
for (; i < stringLen; i++) {
let ch = str[i].toLowerCase();
if (ch !== " ") {
if (result[ch] === undefined) {
result[ch] = 1;
} else {
result[ch]++;
}
}
}
Use comments to explain why you do what you do (if not obvious), and not what you do - the code shows that.
The whole ting could also be done like this:
function characterCounter(str) {
if (!str) {
return "String input required";
}
if (typeof str !== "string") {
str = str.toString();
if (typeof str !== "string")
return "Invalid input";
}
const result = {};
for (var i in str) {
var ch = str[i].toLowerCase();
if (ch !== " ") {
if (result[ch] === undefined) {
result[ch] = 1;
} else {
result[ch]++;
}
}
}
return result;
}