8 ms·

The code quality is amazing. Good job.

saberience 8y ago
On the contrary, I thought the code was terrible quality. If someone wrote this code at my company they would be fired, or more likely not hired in the first place.
- nvssj 8y ago
  What makes you say so? Can you give examples?
  - saberience 8y ago
    Well, for a start... having a single file called "server.rs" which is 2256 lines long is already disqualifying. This file, code structure would be incredibly poor for developer efficiency. It just grows and grows and does all the things while having a vague sounding name like "server.rs" Why is server.rs doing "EditSite." What does the name EditSite mean anyway? Poor code reuse, same bits of code copy and pasted all over instead of being refactored out into their own methods. Methods which are doing too much different crap. The single responsibility principle is broken everywhere. Imagine trying to write unit tests for "Perform for EditPost", this method is doing far too much and would be a pain in the ass to fully test. If you're making a method called "EditPost" it should only be doing the actual post editing, not making connections, or user validation or checking for a ban. These should happen in a method which only handles connections, or a method which is called "ValidateUser" which is properly re-used. Generally speaking, the structure as a whole isn't very readable, poor names, methods and files which are doing too much. The project wouldn't scale well with a bigger team or if it grew in terms of codebase size.
    - 1ba9115454 8y ago
      There are a few opportunities for code generation here. The server.rs looks like an implementation of RPC over websockets. So a refactoring might be to model the interface using something like https://grpc.io/ https://grpc.io/ or any other IDL then code generate Rust server stubs and Typescript client stubs. Cargo has a mechanism for this. https://doc.rust-lang.org/cargo/reference/build-scripts.html https://doc.rust-lang.org/cargo/reference/build-scripts.html The other area where there is some boiler plate is the Diesel ORM. Diesel does generate some code ie. schema.rs but you are left to write the structs that implement these as well as Form structs for passing data to Diesel.
    - A2017U1 8y ago
      What is the max lines in a single file for a developer "to get fired" at your company?
      - nprateem 8y ago
        573
      - Zhyl 8y ago
        The company has a shell script running in Cron. If it finds a source file with more than 1000 lines it deletes the author from the payroll.
        nercury 8y ago
        Sadly this cron has fired the shell script author, and no one dares to touch it.
        fmoralesc 8y ago
        No, it is written in APL for extra conciseness. When they wrote it in shell it deleted the authors from the payroll.
        GoblinSlayer 8y ago
        The author is entire team.
      - megous 8y ago
        It's a ridiculous metric.
      - Insanity 8y ago
        I do have to say, 2k lines for a single file is scary sounding to me. It would probably make me consider splitting it up. BUT, sometimes things to just logically belong together and having one file to find them is just easier. It's a rediculous metric to use.
        twic 8y ago
    - eeZah7Ux 8y ago
      > 2256 lines long is already disqualifying I'd love to hear a justification for this idea.
- MarvelousWololo 8y ago
  That's a terrible, sad and rude comment that does not belong in HN. Instead of just criticize you could offer a constructive review. I hope English is not your native language and it has a less aggressive tone in the translation.
twic 8y ago
Given the response to this, i would be interested to hear why you thought the code quality was amazing. We seem to have two people with different ideas about code quality, and perhaps we would all learn something by investigating that difference.
- s_y_n_t_a_x 8y ago
  I was mainly commenting on the frontend as I'm not well-versed in Rust, my dissenter seems to have a problem with the backend and the organization of the project, which seems unfair since it's in it's infancy, you can always restructure a project and reorganize code. I was commenting on the code, not the files. As for the frontend, it's all typed, and very minimal. Very rarely do we see frontend code that does just what it needs to do and nothing else. The fact that they achieved a "React-like" (Inferno) environment and managed to keep the frontend under 80KB is a feat. That's not to say it's perfect, but it's not finished. I was just very surprised to be able to jump right into the code and see what was going on without wading through boilerplate.
  - s_y_n_t_a_x 8y ago
    I'm not sure why my compliments are being downvoted. Do ya'll hate the code that much?
    - twic 8y ago
      Forget it Jake, it's Hacker News.

On the contrary, I thought the code was terrible quality. If someone wrote this code at my company they would be fired, or more likely not hired in the first place.

nvssj 8y ago
What makes you say so? Can you give examples?
- saberience 8y ago
  Well, for a start... having a single file called "server.rs" which is 2256 lines long is already disqualifying. This file, code structure would be incredibly poor for developer efficiency. It just grows and grows and does all the things while having a vague sounding name like "server.rs" Why is server.rs doing "EditSite." What does the name EditSite mean anyway? Poor code reuse, same bits of code copy and pasted all over instead of being refactored out into their own methods. Methods which are doing too much different crap. The single responsibility principle is broken everywhere. Imagine trying to write unit tests for "Perform for EditPost", this method is doing far too much and would be a pain in the ass to fully test. If you're making a method called "EditPost" it should only be doing the actual post editing, not making connections, or user validation or checking for a ban. These should happen in a method which only handles connections, or a method which is called "ValidateUser" which is properly re-used. Generally speaking, the structure as a whole isn't very readable, poor names, methods and files which are doing too much. The project wouldn't scale well with a bigger team or if it grew in terms of codebase size.
  - 1ba9115454 8y ago
    There are a few opportunities for code generation here. The server.rs looks like an implementation of RPC over websockets. So a refactoring might be to model the interface using something like https://grpc.io/ https://grpc.io/ or any other IDL then code generate Rust server stubs and Typescript client stubs. Cargo has a mechanism for this. https://doc.rust-lang.org/cargo/reference/build-scripts.html https://doc.rust-lang.org/cargo/reference/build-scripts.html The other area where there is some boiler plate is the Diesel ORM. Diesel does generate some code ie. schema.rs but you are left to write the structs that implement these as well as Form structs for passing data to Diesel.
  - A2017U1 8y ago
    What is the max lines in a single file for a developer "to get fired" at your company?
    - nprateem 8y ago
      573
    - Zhyl 8y ago
      The company has a shell script running in Cron. If it finds a source file with more than 1000 lines it deletes the author from the payroll.
      - nercury 8y ago
        Sadly this cron has fired the shell script author, and no one dares to touch it.
      - fmoralesc 8y ago
        No, it is written in APL for extra conciseness. When they wrote it in shell it deleted the authors from the payroll.
      - GoblinSlayer 8y ago
        The author is entire team.
    - megous 8y ago
      It's a ridiculous metric.
    - Insanity 8y ago
      I do have to say, 2k lines for a single file is scary sounding to me. It would probably make me consider splitting it up. BUT, sometimes things to just logically belong together and having one file to find them is just easier. It's a rediculous metric to use.
      - twic 8y ago
  - eeZah7Ux 8y ago
    > 2256 lines long is already disqualifying I'd love to hear a justification for this idea.
MarvelousWololo 8y ago
That's a terrible, sad and rude comment that does not belong in HN. Instead of just criticize you could offer a constructive review. I hope English is not your native language and it has a less aggressive tone in the translation.

twic 8y ago

Given the response to this, i would be interested to hear why you thought the code quality was amazing. We seem to have two people with different ideas about code quality, and perhaps we would all learn something by investigating that difference.

s_y_n_t_a_x 8y ago
I was mainly commenting on the frontend as I'm not well-versed in Rust, my dissenter seems to have a problem with the backend and the organization of the project, which seems unfair since it's in it's infancy, you can always restructure a project and reorganize code. I was commenting on the code, not the files. As for the frontend, it's all typed, and very minimal. Very rarely do we see frontend code that does just what it needs to do and nothing else. The fact that they achieved a "React-like" (Inferno) environment and managed to keep the frontend under 80KB is a feat. That's not to say it's perfect, but it's not finished. I was just very surprised to be able to jump right into the code and see what was going on without wading through boilerplate.
- s_y_n_t_a_x 8y ago
  I'm not sure why my compliments are being downvoted. Do ya'll hate the code that much?
  - twic 8y ago
    Forget it Jake, it's Hacker News.