Hacker News Reader

Hacker News

Comment by on_and_off | Hacker News Reader

Parent Full thread

on_and_off

Interesting that the official Telegram client gets mocked a lot for how bloated its code is.

And it absolutely is, from a 2 minutes look at their code :

- their chat activity is 12000 lines of code : https://github.com/DrKLO/Telegram/blob/master/TMessagesProj/...

- it looks like they have copy pasted tons of Android library like exoplayer directly in their repo

It does work very well though !

I guess that they have an extremely small team (or just one persone) and it is their first Android project.

It looks like they have acqui-hired a competing chat client (Telegram X), so it looks like they have a solution to clean this mess.

View on HN

Woah, that's some lazy code in ChatActivity.java. Functions with 30+ conditionals depending on tons of external state. That must be a nightmare to debug and impossible to test.

I've seen some huge Android java functions where everything gets stuffed into the Fragment creation/update/etc API functions... but this is one just stuffed everything in them.

It's just asking for bugs and security issues.

Edit: the commit history all comes the same developer, each titled with a generic "Bug fixes" commit and no description of the changes. Seems like a single guy is just cowboying the whole App, it's not a team project at Telegram. Which explains the above... https://github.com/DrKLO/Telegram/commits/master/TMessagesPr...

However it feels very good to use. Cases in which something gets weirdly stuck (happens in other apps) are virtually non existent. It is fast and quite reliable. Perhaps surprisingly so.

I'd be more concerned about the developer side, particularly new developers and OSS contributors, the wide attack surface introduced by such complexity, long-term inflexibility, impossible to track down bugs, etc more than anything. I'm sure they have plenty of people to QA from the user perspective, just clearly not the code.

Maybe the developer was pressured to focus on releasing quickly over code quality...yada yada. The typical startup developer story. But not a story that I like to find in my security-focused software.

I guess the point is that you can get away with such sloppy code as long as it is a solo project of a relatively small size.

I am not sure at which point it becomes too big and new features and bugs are just impossible to handle.

It might have been the right choice for Telegram though. Their app feels snappy, so the 'only' big downside is that now it looks like they are going to have to thrown away this entire code base and replace it with the one from Telegram X (a third party client they bought)

Don’t worry, most current message apps have code that horrible.

Be it matrix simply writing a HashMap into a file as "database" replacement https://github.com/matrix-org/matrix-android-sdk/blob/master...

Or Signal simply taking apart Android system structs with JNI, to get a file descriptor’s owner https://github.com/signalapp/Signal-Android/blob/master/jni/...

There’s lots of suboptimal code in messaging apps. All of them.

Why do you need a database over a serialized object like a hashmap file if there is only one user and one device?

Because writing the entire dataset back out on every addition or removal is extremely wasteful.

The existing database implementations in the system do much less writes.

Doing such large, constant writes not only is slower, it also degrades the NAND.

Thank you

That's the reason they are working on the next-gen mobile client now, code-named Telegram X [1] [2]. In my tests it appears to be faster and lighter, I can't speak for the code though as it hasn't been released yet.

1. https://itunes.apple.com/us/app/telegram-x/id898228810?mt=8 2. https://play.google.com/store/apps/details?id=org.thunderdog...

Curious why typecast if instanceof is checked already ? Wont the compiler know of this already ?

  if (view instanceof ChatMessageCell) {
  .......                    
   ChatMessageCell cell = (ChatMessageCell) view;
  ....
  }

It's essentially just how Java works, and isn't that uncommon in statically typed procedural languages.

The `view` variable is still a reference of the parent type (Say `MessageCell` for now, can't be bothered to find that part in the code), and `MessageCell` references can only do certain things.

It's possible to check what the type of the underlying object is with `instanceof`, but that doesn't change the type of the reference.

If you want to do things that only a `ChatMessageCell` can do, you need to make a new reference of that type.

It's Java, so no.

the java compiler does not do that unfortunately,

Kotlin would do what they call a 'smartcast' : ie if you did an instanceof if the if, in the corresponding {}closure you know the type of this object.

The whole code is probably unmaintanable for anybody other than it's original writer (and even for him, there must be a limit to how much code you can fit in your head).

I would love to work for Telegram, it is one of the products I use daily .. but if I had to handle this codebase, it would be a rare case of 'total rewrite or I am out'.

Incidentally, it looks like they are going to do this full rewrite. Telegram seems to have acqui-hired the creators of Telegram X and are prepping it up as a replacement.

I believe this is called “type narrowing”, and no, Java doesn’t have this.

That sounds like a disturbingly huge attack surface...

Reply on news.ycombinator.com