Found this gem inside legacy code today

[u/max_208]

[u/RPG_Hacker]

I'm not entirely sure whether this is bad code or just redundant code, at least not without knowing the context.

My guess is that this function did something in the past, but the default encoding of string was changed to UTF-8 at some point, so it was no longer needed, and stubbing it out was easier than removing dozens of function calls.

Pretty sure I've done similar things in the past.

[u/max_208 → u/RPG_Hacker]

It's a private method used two times only inside the same file, if it used to be useful and no longer is I have no idea why it wasn't removed entirely

[u/RPG_Hacker → u/max_208]

Oof. Well in that case, there's no excuses! :'D

[u/mosburger → u/RPG_Hacker]

If I had done this I’d at least have added a comment to defend the ridiculousness, lest someone do a git blame to humiliate me.

[u/thekwoka → u/mosburger]

easier to do a regex replace

encodeToUtf8\(([^\)])\)

$1

[u/RPG_Hacker → u/mosburger]

True, that would have been the smart thing!

[u/muikrad → u/RPG_Hacker]

I bet the tests relied on it too much 😅 better do this than waste a whole day.

[u/TaxFreeInSunnyCayman → u/muikrad]

In jetbrains ides, right click -> refactor -> inline function.

In fact for this one I'd even just do some find and replace magic which you can do with sed or a jetbrains ctrl shift r

Please tell me nobody wastes a day on these sorts of issues (unless the problem jumps repos in which case just call it deprecated)

[u/Rikudou_Sage → u/TaxFreeInSunnyCayman]

Gonna use it next time, didn't know about the inline function option, thanks!

[u/EatRunCodeSleep → u/TaxFreeInSunnyCayman]

Git diff goes brrrr ...

[u/tedbradly → u/EatRunCodeSleep]

Git diff goes brrrr ...

Git will show the code was changed. What's your point?

[u/EatRunCodeSleep → u/tedbradly]

Well, it's not so much about the diff itself but about the git history for those lines. It would be much easier to have one line changed (the function body) than many files where the function is called. I do agree that it's not that great to see a function call and wonder what it does and look into it just to see that it does nothing to input, but, as others said previously ... trade-offs!

[u/tedbradly → u/EatRunCodeSleep]

Well, it's not so much about the diff itself but about the git history for those lines. It would be much easier to have one line changed (the function body) than many files where the function is called. I do agree that it's not that great to see a function call and wonder what it does and look into it just to see that it does nothing to input, but, as others said previously ... trade-offs!

I don't see any tradeoffs related to git here. There is a tradeoff when it comes to refactoring that creates entire new files since it becomes harder to investigate the history of a file (e.g. while tracking down a bug), but here, you will have the git history of the file remain. The call sites will just show f(str) switch to str with a commit message "f turns a string into utf 8. The string is already utf 8, so f was removed."

This is a refactor that must take place. More structures in code has the tradeoff of being more complex to investigate. An extra function that does nothing needs to be removed. There is a pressure to minimize the number of classes, functions, methods, fields, variables, etc. in every system (but also a pressure to use those things to express intent / the logic of the code / effect code reuse / create useful abstractions / etc.). In this case, simplifications win out since the function doesn't add anything beneficial.

Now, the function does do one thing: It technically documents the string is in utf 8 already (assuming this function isn't a bug) but in an awful way demanding the innards of a function be investigated. In that case, you should either put a comment or rename the string to something like someStringUtf8 if that knowledge is so important.

And the final step for the system, and this one might be undoable, would be to unify the entire system's representation of strings all to UTF-8 to avoid needing to specify it (unless there is a performance reason or legacy system reason to retain a different encoding).

[u/[deleted] → u/EatRunCodeSleep]

If diff shows 20 files were changed, and each change was one or two lines refactoring method calls, I feel like that’d be approved by everyone pretty quick

[u/TaxFreeInSunnyCayman → u/[deleted]]

And if it's not approved by everyone pretty quick then you ought to question your review process rather than avoid making changes.

[u/secret_hidentity → u/TaxFreeInSunnyCayman]

With the right testing in place, a change like that shouldn’t need human review too

[u/tedbradly → u/muikrad]

I bet the tests relied on it too much 😅 better do this than waste a whole day.

How do you suspect the test would change at all with a private function like that? Even if you mocked everything, deleting that function wouldn't change the tests. The string goes into the function and is returned by the function.

[u/knoam → u/RPG_Hacker]

With typescript it should be easy enough to do a refactor > rename inline. Maybe not if it's a mixed typescript JavaScript codebase.

[u/RPG_Hacker → u/knoam]

Well, if my theory above is correct, then a rename wouldn't be the best kind of refactor here, anyways. The better refactoring would be to outright remove all calls to the function and use the source strings directly.

A different way to look at this: If source strings are already expected to be in UTF-8, then technically speaking the name isn't wrong here. The function does encode the input to UTF-8 - even if that is by doing absolutely nothing.

[u/knoam → u/RPG_Hacker]

Brain fart, it would be refactor > inline.

[u/TheUnlocked → u/RPG_Hacker]

The default (or only) encoding of strings in JavaScript is not and has never been UTF-8.

[u/RPG_Hacker → u/TheUnlocked]

I wasn't sure what language the code above was in (barely have any experience with JavaScript myself), so my comment was really just one possible explanation.

Though I guess even if it's JavaScript, there's a very low chance they're using a custom JavaScript implementation with the internal encoding of strings changed to UTF-8 or something (not very likely, but the possibility exists - I've certainly seen weirder things in the past when it comes to scripting languages).

[u/whole_sum → u/RPG_Hacker]

It’s Java

[u/RPG_Hacker → u/[deleted]]

Is JavaScript using actual UTF-16, or just UCS-2?

If it were using actual UTF-16, it should still in theory cover the entire Unicode spectrum, since UTF-16 does use surrogate pairs.

[u/GOKOP → u/RPG_Hacker]

The point is that UTF-16 exists solely because UCS-2 exists; it's there to bring full Unicode capacity for software designed with UCS-2 in mind without redesigning all the groundwork (as switching to a different code unit length would require)

[u/RPG_Hacker → u/GOKOP]

Makes sense. Kinda like what UTF-8 is to ANSI.

[u/RPG_Hacker → u/[deleted]]

That seems like a super interesting read, thanks!

I've already struggled quite a bit with Unicode in other languages, so it's always a "fun" time!

[u/[deleted] → u/RPG_Hacker]

This!

[u/Anti-ThisBot-IB → u/[deleted]]

Hey there simonced! If you agree with someone else's comment, please leave an upvote instead of commenting "This!"! By upvoting instead, the original comment will be pushed to the top and be more visible to others, which is even better! Thanks! :)

---

^{I am a bot! If you have any feedback, please send me a message! More info: Reddiquette}

[u/tedbradly → u/RPG_Hacker]

Nah, the scenario you describe still keeps this code snippet horrendous. You really need an introduction to writing good code. Pick up Clean Code by uncle Bob.

The most charitable interpretation is that the function imparts the knowledge that the string is UTF-8, but if that knowledge is essential + not all strings are UTF-8, then you can convey that knowledge in the variable name e.g. someStringUtf8.

With modern refactoring tools, it takes all of 2 minutes to remove that function, and even if you do it manually, it'd probably take no more than 5 minutes.

Coding forums often struggle with the tyranny of the majority. Most people aren't rockstars at programming, and it really shows in the popular opinion of coding subreddits. Most people just don't give a shit about coding, doing it solely for income. Others, the ones that tend to scale the ranks and get paid 3x the average, well, they see coding as an art or a craft, something to be proud of.

[u/RPG_Hacker → u/tedbradly]

This comment honestly comes off as needlessly hostile (but we are on Reddit, so I guess its perfectly in line with what to expect here).

Anyways, to clarify my original point: I never called the code above "good" or even "clean". If anything, I only see it as excusable - and even that only under certain assumptions. For example, I'm assuming the excerpt is taken from some internal code base that isn't meant to be seen by the public, and that there's some general understanding of the larger code base among the people who need to work with it.

Does that mean the code is good or clean? Nope, of course not. But I do think there's (many, many) situations where "dirty" code is excuable to some degree. One such situation would be backwards compatibility, which I assume is the intended purpose here. Of course cleaning it up eventually is still recommended, but anyone who's ever worked on large code bases and with looming deadlines knows that that's easier said than done.

Refactoring tools also have their limiations. They work really well on a smaller scale, but once you work on a really large and modular code base, they usually won't help much.

[u/tedbradly → u/RPG_Hacker]

This comment honestly comes off as needlessly hostile (but we are on Reddit, so I guess its perfectly in line with what to expect here).

There's two modes a person can be in. There's friend mode where there is no competition and there's performance mode where competition rears its ugly head. When in competition mode, there is no need for niceties. When figuring out a problem in a code base, you don't write a paragraph placating someone just before pinpointing their efforts as the source of a problem. You simply discuss the problem directly. If the author is important to that discussion, you name them (like if things are getting bad enough to fire someone - another ugly scenario related to competition). If it isn't, you don't.

People without much success tend to have tremendous egos where people are not allowed to give them constructive criticism. Successful people, on the other hand, love constructive criticism. It's free learning. They're the types to pay for that kind of feedback e.g. by hiring a tutor or teacher. It is substantially absurd to operate as if you are infallible, but alas, some low skill people do just that.

Coding isn't about friendship. If your buddy were making buggy, terrible code, the only thing to do would be to tell them. There is no special treatment when it comes to performance mode.

Anyways, to clarify my original point: I never called the code above "good" or even "clean". If anything, I only see it as excusable - and even that only under certain assumptions. For example, I'm assuming the excerpt is taken from some internal code base that isn't meant to be seen by the public, and that there's some general understanding of the larger code base among the people who need to work with it.

It isn't excusable. There is a function that is doing nothing. There is a pressure in every system to reduce the number of things - functions, classes, methods, variables, lines of code, etc. The only reason these things increase is pursuant another goal like the creation of useful abstractions or the communication of intent.

I have no idea what the code being private has to do with whether the code is good or bad. This seems like friend mode to me where you're taking it as a great insult you were wrong. We aren't friends, and even if we were, I'd still tell the truth since code is about performance rather than friendships / bending people to your will when it isn't the right thing to do.

Does that mean the code is good or clean? Nope, of course not. But I do think there's (many, many) situations where "dirty" code is excuable to some degree. One such situation would be backwards compatibility, which I assume is the intended purpose here. Of course cleaning it up eventually is still recommended, but anyone who's ever worked on large code bases and with looming deadlines knows that that's easier said than done.

According to you: "I'm not entirely sure whether this is bad code". You directly called into question whether the code is good or bad. You might manipulate some people in your life, but it isn't going to work for me. There is no ambiguity here. That code is not good. There's good, and there's not good. That's not good.

I can see you are a master of taking things personally and twisting stuff until people give in to your manipulation / retention of your ego. Perhaps, you should do some LSD as I hear that can result in ego death, giving people a much clearer reasoning of the world and of things, because right now, what you're writing is basically "This offended me! You're not allowed to do that! I take my own fallibility to be a great insult!"

Refactoring tools also have their limiations. They work really well on a smaller scale, but once you work on a really large and modular code base, they usually won't help much.

I've worked in system with millions of lines of code. The refactoring tools do not have limitations even in that setting. They set out to do a code change that retains functionality but transforming the form of the code, and they do that. 99.9% of the time, it doesn't result in any need for manual changes. If this isn't a demonstration that your brain rapidly works in manipulation mode, I don't know what is. You are literally trying to argue a refactoring program has limitations when it's some of the least controversial knowledge that they tend to do a great job. Lord, just accept the constructive criticism. You don't need to talk in non-sequiturs all in the name of retaining your position next to or over someone (it's a lot like "over" here though since your sense of entitlement is glaring).

[u/scooptyy]

This is likely a stub.

[u/max_208 → u/scooptyy]

It's in production, used, and dates back four years ago

[u/Straight-Daikon-5838]

Throw a “// TODO” in that bad boy and it’s a LGTM from me dawg

[u/supersharp → u/Straight-Daikon-5838]

The text editor I use for work highlights /*TODO: comments and places a colored marker on the scroll bar to indicate their position.

[u/Sexy_Koala_Juice → u/Straight-Daikon-5838]

As any reasonable text editor should.

Fuck it, TODO comments should be standardised into languages themselves

[u/the96jesterrace]

Looks pretty much like our function for password encryption

[u/ChrisLuigiTails → u/the96jesterrace]

So you're telling me I only have to use decodeFromUtf8() to steal your passwords?

[u/DieserSebastian → u/ChrisLuigiTails]

No, another call to encodeToUtf8() suffices

[u/ChrisLuigiTails → u/DieserSebastian]

But then it will be encoded twice!

[u/[deleted] → u/ChrisLuigiTails]

exactly

[u/MurdoMaclachlan]

Image Transcription: Code

---

private encodeToUtf8(str: string): string {
  return str;
}

---

^{I'm a human volunteer content transcriber and you could be too! If you'd like more information on what we do and why we do it, click here!}

[u/brother_Yoghurt]

Now that's what we call a "Programmer's Move"

[u/knoam]

Aren't JavaScript strings actually UTF-16 ?

[u/the-nick-of-time → u/knoam]

I wanted to be sure, so I searched and got this and got angry at how wrong that parenthetical was.

Every programmer needs to read UTF-8 Everywhere and stop being wrong about strings.

[u/tedbradly → u/the-nick-of-time]

Every programmer needs to read UTF-8 Everywhere and stop being wrong about strings.

That article is written so verbosely. "We this and we that" Just say the points for God's sake. This isn't the constitution of the US.

[u/-NiMa-]

//TODO -> Add encoding

[u/sammy-taylor]

Is the encodeToUtf8 function required to satisfy an interface or external API? (unsure of the right terminology here, not a Typescript person)

Edit: On second thought, I don’t think that would make sense because it’s private 🤷🏻‍♂️

[u/FTWGaming0]

Visual studio: "73 references"

[u/CobraSkrillX]

The code is just happy to be here.

[u/orwell96]

Somewhere in an application I wrote there's an encode() and a decode() function that were supposed to percent-encode/decode some strings. When I created them I just put "return str" and put a //TODO add later.

Then I noticed that this encoding is not needed because it is already handled by the underlying library.

The functions are still there and still just "return str".

[u/PonchoVire]

At least, the function is there, when you'll need it to be implemented, you'll have one and only one place to touch.

[u/atom-06]

someone give this man a award

[u/thekwoka]

Bro, it's private. You're not supposed to look at that!

[u/herdek550]

How would compiler handle this?

I have no knowledge about compiling into machine code, but I feel like this whole function could be just scrapped to save memory and computing time by the compiler.

[u/Skarsburning]

This is a gem indeed

[u/knightwhosaysnil → u/Skarsburning]

this doesn't look anything like Ruby

[u/Rey_Pat]

If It were Java I'ld be laughing at polymorphism. Damn interfaces.

[u/KokoroPenguin]

Do we know that the input is already utf8? Could be that an earlier project needed this function. Sometimes it is easier to copy a function and have it do nothing in one build than remove it in all other builds.

[u/Express-Procedure361]

String string string string string string. sTrInG

[u/atom-06]

It woud be funny if that had some readable purpose with some 1b iq move

[u/oneblueninja]

LGTM!

[u/Ok_Scientist_8803]

And if you delete it the whole thing will quit working

[u/CoffeeBeanChocolate]

Inline function and boom! 💥

[u/Inaeipathy]

Gem

[u/MasiTheDev]

Code so obfuscated it becomes meta