Style
- Javascript uses camelCase for naming. Don't use snake_case.
- Use functions to reduce repetition.
- Use the shortest or simplest form. Eg you define RegExp as strings and then create them when needed. It is simpler to define the RegExp directly, saving the need to add the extra control character delimiter
\\.
- Don't duplicate code. You test for
mode not equal to "file" and "folder" then you test for mode equal to "file" then "folder" where a final else clause will do the first test without extra overhead. However I would wonder why the function is called with an unknown mode?
- Use arrow functions when you can to reduce code size and thus increasing readability.
- Don't assign variables with unused content that will be replaced further in the code. Eg
let suffix = ""; should be let suffix;.
- Learn to use the correct scope variable. In function scope you should use
var not let and should be defined at the top of the function
- In JavaScript for the browser an element is a very specific type of object, generally when iterating a nondescript array of items we call the item
item, using element is misrepresenting what it is.
Code
- You say legacy, but there is no clue as to which system you are converting to.
- Your code comments indicate that
mode is * @param {string} mode "file" / "folder" as a string yet no indication that it can be another value and such value would modify the return type.
- The documentation comments give no return value and type. This is a particularly glaring omission as the return type can be two different types, a
string or a boolean. Parcial, or incorrect documentation is worse than no documentation.
- The differing return types based on
mode is very problematic, and means that you need to add code to the calling function to test the return. This means that there is an indirect duplication of the test, in the function and the calling function. You may as well have the calling function determine if it should call the function in the first place and hive the function only return a fixed type.
- Convert to the file/dir name to lowercase as first step.
- Is the extension to keep its case?? I would think not (though this is a guess on my part)
- There is no need to add the grouping to the regExp's
\()\ as you don't use them.
- There is no need to create a new variable to hold
object_name, you can use it (the argument) rather than create a new one.
- When you strip the extension (suffix) from the filename, you use a global regExp. It is possible for filenames to include many
"." eg the filename "myfile.common.com" would be replaced with "myfilemon" rather than "myfile.common". You could store the split name and join it after popping the extension, or just trim the string.
- Names.
object_name is a bit vague, its a filename or directory name, name is ma ybe a better name?mode a little odd, maybe type is a better description?suffix though not incorrect the more common name is fileExtension, fileExt, extension or ext.retvar Should be retVar (camelCase) but that said it is a bad name. It is a var (well technically a let) so you have included a obvious implied attribute of the variable. Generally for non specific return variable we use result or in this case may be sanitizedName would be better? (or see example where name is used to hold the result)element is a DOM object, can be confusing so maybe a better name is item?
The rewrite
Addressing most of the points above the rewrite only returns a string (mode now called type is "file" or it defaults as "folder"). Converts the extension to lowercase. Use RegExp literals rather than create them via instantiation. Use arrays to hold replacement arguments so that the spread operator can be used to call replace.
Please Note that I had to remove many / from the reg expressions, I did not test the edited regExps so the last one may have errors regarding what it replaces.
/**
* @description Sanitizes a folder or file name to work with #name of legacy system
* Legacy system unknown
* @author BM67
* @param {String} name The string to sanitize
* @param {String} type Optional "file" or default is assumed "folder"
* @return {String} The sanitized name
*/
function sanitizeFolderOrFile(name, type) { // type is "file" all else is "folder"
var ext = ""; // adds the default at return if type is folder
name = name.toLowerCase(); // move below following statement if case must be maintained
if (type === "file") {
const parts = name.split(".");
ext = "." + parts.pop();
name = parts.join("."); // Could join with "_" as you replace it later
}
[[/[+\&]/g, "u"], [/ä/g, "ae"], [/ö/g, "oe"], [/ü/g, "ue"], [/ß/g, "ss"],
[/[ \`\´\?\(\)\[\]\{\}\/\\$\§\"\'\!\=\-\.\,\;\:<>\|\^\°\*\+\~\%]/g, "_"]
].forEach(item => name = name.replace(...item));
return name + ext;
}
Or for those that like it spaced out (Note also keep extension case)
function sanitizeFolderOrFile(name, type) {
var ext = "";
if (type === "file") {
const parts = name.split(".");
ext = "." + parts.pop();
name = parts.join("_");
}
name = name.toLowerCase();
[
[/[+\&]/g, "u"],
[/ä/g, "ae"],
[/ö/g, "oe"],
[/ü/g, "ue"],
[/ß/g, "ss"],
[/[ \`\´\?\(\)\[\]\{\}\/\\$\§\"\'\!\=\-\.\,\;\:<>\|\^\°\*\+\~\%]/g, "_"]
].forEach(item => name = name.replace(...item));
return name + ext;
}
And the ES5 version
function sanitizeFolderOrFile(name, type) {
var parts, ext = "";
if (type === "file") {
parts = name.split(".");
ext = "." + parts.pop();
name = parts.join("_");
}
name = name.toLowerCase();
[
[/[+\&]/g, "u"],
[/ä/g, "ae"],
[/ö/g, "oe"],
[/ü/g, "ue"],
[/ß/g, "ss"],
[/[ \`\´\?\(\)\[\]\{\}\/\\$\§\"\'\!\=\-\.\,\;\:<>\|\^\°\*\+\~\%]/g, "_"]
].forEach(item => name = name.replace(item[0], item[1]));
return name + ext;
}
