garbage string remover with array or object input

Question

This is the second time I am writing any practical JavaScript so it is pretty likely that I have messed something up.

I am beginner, so just need a piece of advice before sharing the code: Do I need to be more focused on writing maintainable and flexible code or do I need to be more focused on writing as concise code as possible? Personally I am working for writing maintainable and flexible code, but I feel like the web is about transfer of data between the server and the client, so more code means slower transfer speed, which means rough user experience. But anyways, because minification can be done with the help of git, that is why I am practicing for maintainable and flexible code. Is it right thing to do?

This code only removes strings that have "garbage" (case-insensitive) in them. No matter how deep they lie.

let dirtyArray = [{
    id: 1,
    listOfItems: ["item1", 34, null, 56, "totalgarbage", null],
    garbage: "trash"
  },
  [2, "kPOP", undefined, null, 23],
  45, ["NoLeSsThAnGaRbAgE"],
  "thisIsGarBageArray"
]

function containsGarbage(something) {
  if (typeof something === "string") return something.toLowerCase().includes("garbage")
  if (Array.isArray(something)) return arrayCleaner(something) // don't know if this will cause any unforeseen problems ... it looks like the function is returning somthing, while it really isn't.
  if (something instanceof Object) return objectCleaner(something)
}

function arrayCleaner(arr) {
  arr.forEach((item, index, array) => {
    if (containsGarbage(item)) delete array[index]
  })
}

function objectCleaner(obj) {
  for (let [key, value] of Object.entries(obj)) {
    if (containsGarbage(value) || containsGarbage(key)) delete obj[key]
  }
}

containsGarbage(dirtyArray)
console.log(dirtyArray)

The main functionality is performed by the three functions, other code is just an example to show what it does.

One pitfall is that while deleting an element, the array substitutes the deletion with undefined. That might not be a problem practically because I don't think someone keeps their arrays this dirty, but given that there were pre-existing undefineds, that might be a problem. Another pitfall is mention in the code.

Answer 1

I am practicing for maintainable and flexible code. Is it right thing to do?

Yes!

Delivering loads of javascript to the client can be slow, but the way to solve is this is not by writing worse code. You solve it by minifying/uglyfying, chunking, tree shaking, and by not including lots of third party code that you haven't determined is worth the size. Writing bad code (dense, hard to read) makes it harder to understand and harder to refactor, and might ironically make it longer in the end.

I have just a couple of comments, and it seems you are aware of them already:

• containsGarbage reads like something that just reads the input and returns a boolean, but in fact it mutates the input and possibly returns undefined (the pitfall you mentioned). A better name would be removeGarbage. I would also make it return true or false explicitly, rather than relying on undefined.
• You can use array.filter() to avoid the <empty value>'s laying around.

Also, If you want to write maintainable code I cannot recommend pure functions enough (no input mutations or side effects allowed). The performance is sometimes worse, but often acceptable.

For instance:

const isGarbageString = (something) =>
    typeof something === 'string' && something.toLowerCase().includes("garbage")

const removeGarbage = (something) => {
    if (Array.isArray(something)) return removeGarbageFromArray(something)
    if (something instanceof Object) return removeGarbageFromObject(something)
    return something
}
const removeGarbageFromArray = arr => arr
    .filter(item => !isGarbageString(item))
    .map(removeGarbage)

const removeGarbageFromObject = obj =>
    Object.entries(obj).reduce((newObj, [key, value]) => {
        if (isGarbageString(key)) return newObj
        if (isGarbageString(value)) return newObj
        return { ...newObj, [key]: removeGarbage(value) }
    }, {})

Here removeGarbage is pure, and that is a win for maintainability and readability since you can always call pure functions without worrying about unintended consequences. The performance of this example is not the best though, so maybe don't use this for millions of large objects ;)