3 ms·

Interesting product. I'm reading through the source and... Woah... it is all in one single file with zero unit tests. Of course there are many ways to skin a c

Interesting product. I'm reading through the source and... Woah... it is all in one single file with zero unit tests. Of course there are many ways to skin a cat, this seems like a very odd choice for organizing a database codebase. Some of the methods like cluster.HandleClientConnection() are crazy long for loop + if/else + switch + goto! I know that you've said this is just kind of in your best interests codebase, but if I was planning on making any sort of dependencies on this project, I'd probably shy away from it because it seems like unmanageable spaghetti.

throwup238 3y ago
> unmanageable spaghetti. It's not even 3,000 lines. That's not a lot of spaghetti. cluster.HandleClientConnection looks like a standard network service event loop, except the code is inline instead of spread across cluster.HandleSyncData, .HandleDelete, etc. The lack of tests though...
- alexpadula 3y ago
  If you guys really want me to post the tests I will. No problems!
  - alexpadula 3y ago
    There is a lot of them so I will create a new repository for them, possibly; Or commit them to the core repo. I’ll see. I did have tests within the main repo at one point but I took them out before v2.0.0.
    - latchkey 3y ago
      Taking the tests out is like doing the homework and not turning it in for grading.
      - alexpadula 3y ago
        I mean you feel pretty good about removing them after hundreds of automated tests runs and hundreds of hours of manual testing. I could be biased in that I will put back tests for the sanity of you all :). I do understand for contribution sake to have them there.
        latchkey 3y ago
        I don't necessarily look at tests as something for the past, I look at them for the future. If I want to add a feature or change some code around to fix something (like, say, migrate from one giant file to a bunch of individual files), then I want some sort of assurance that I haven't broken anything. That's what tests give you. If you have someone contributing to your project, who doesn't necessarily have the whole 3,000 lines in their head, then tests give them the ability to make those contributions safely, without breaking anything. It is all about doing things as standard as possible. Lowest common denominator for every one looking at the code base. As soon as you start to do things strangely in code (like a big single file with no tests), then people start to curse you. "What were they thinking when they wrote this?!!#@#!@#". I try to avoid those situations. Tests also help document "the why". They become a history for changes over time. I could start to fix something that seems like a bug or strange design decision and look at the tests and realize that things were done intentionally... and give me the "why" they were done that way.
        alexpadula 3y ago
        I agree with you! I'm not going to lie to you the tests are rather different than most. I don't test things say like setting up an initial cluster or node. Those are such basic functionalities that can be manually tested. The system is tested mainly and I say mainly on: - handling the protocol in which the client speaks to the cluster (no need) - handling the protocol in which the cluster speaks to the node (no need) - the query language (when testing the query language we are testing the above 2) - finding any gaps in mistyping a query that can cause a cluster exception of any kind (this has been insanely gone through, I can't even lol) - concurrency with consistency. One test for example is updating multiple documents multiple times through 100 client connections. I don't see the structure of any main method changing. I just want to add tests must be driven meaning.. With a client. You don't test this system like you do a module in GO. You don't just write one test for CountLog(). If I absolutely had to sure. When you test the client>cluster>node(s). So you always do driven testing as a client or multiple clients.
alexpadula 3y ago
Just because you don’t see tests doesn’t mean that there weren’t any before or now. Really not unmanageable at the size of the code base for me. I do agree that method is long but I didn’t want to spread that and a few others across many others it was a design choice. I agree it could be improved but to not want to implement it because of that is your choice. The code is commented and continuously gets more comments and it’s very easy to follow if you read procedurally.
- latchkey 3y ago
  > for me These keywords are the crux of the issue. What you're expecting is that third parties can read your mind. You have the whole codebase floating in your head so you understand where things are at and how it is laid out. If I'm working with others, or expecting people to pick up my project and either use it or contribute to it, I generally try to make it as easily understood as possible. Common sense is to break the file into manageable sections. Again, tons of ways to skin a cat... but this is a huge red flag, for me.
  - alexpadula 3y ago
    Interesting, well understood. I’ll try my best to rethink that logic and get it even shorter. I have no problems with that!! Thank you for taking a look.
  - alexpadula 3y ago
    I really tried to make everything as understandable as I could truly and I will continuously strive to do so. Mind you I only built it in around 60 days so everything will come :)
    - latchkey 3y ago
      Please don't get me wrong, this is just hopefully constructive feedback from my personal opinion having written A LOT of mission critical golang code. Again, so many ways to skin a cat... if you can convince other people to contribute to your project as it is, then I'm definitely worthy of ignoring. =)
anonzzzies 3y ago
This is spaghetti to you? You must be lucky as this is very clean to what I get to work with daily (and that’s more like millions LoC.
- latchkey 3y ago
  What do you want me to say here? It is 3000 lines, in a single file and no tests. I just said "seems like". In order to comprehend it, I'm going to have to parse the whole thing into memory and figure out what it does. Not impossible, but if I'm looking to just fix a specific bug or contribute a new feature, I'd much rather go to the specific file that feature/bug is in, parse a page or two of code on my screen and work from there. It is just how I work. I'm sorry that you have to deal with something that sounds worse. If it was me, I'd spend a lot of extra-curricular time trying to clean up the mess. In return, that would help me get an even better sense of the code.

throwup238 3y ago

> unmanageable spaghetti. It's not even 3,000 lines. That's not a lot of spaghetti. cluster.HandleClientConnection looks like a standard network service event loop, except the code is inline instead of spread across cluster.HandleSyncData, .HandleDelete, etc. The lack of tests though...

alexpadula 3y ago
If you guys really want me to post the tests I will. No problems!
- alexpadula 3y ago
  There is a lot of them so I will create a new repository for them, possibly; Or commit them to the core repo. I’ll see. I did have tests within the main repo at one point but I took them out before v2.0.0.
  - latchkey 3y ago
    Taking the tests out is like doing the homework and not turning it in for grading.
    - alexpadula 3y ago
      I mean you feel pretty good about removing them after hundreds of automated tests runs and hundreds of hours of manual testing. I could be biased in that I will put back tests for the sanity of you all :). I do understand for contribution sake to have them there.
      - latchkey 3y ago
        I don't necessarily look at tests as something for the past, I look at them for the future. If I want to add a feature or change some code around to fix something (like, say, migrate from one giant file to a bunch of individual files), then I want some sort of assurance that I haven't broken anything. That's what tests give you. If you have someone contributing to your project, who doesn't necessarily have the whole 3,000 lines in their head, then tests give them the ability to make those contributions safely, without breaking anything. It is all about doing things as standard as possible. Lowest common denominator for every one looking at the code base. As soon as you start to do things strangely in code (like a big single file with no tests), then people start to curse you. "What were they thinking when they wrote this?!!#@#!@#". I try to avoid those situations. Tests also help document "the why". They become a history for changes over time. I could start to fix something that seems like a bug or strange design decision and look at the tests and realize that things were done intentionally... and give me the "why" they were done that way.
      - alexpadula 3y ago
        I agree with you! I'm not going to lie to you the tests are rather different than most. I don't test things say like setting up an initial cluster or node. Those are such basic functionalities that can be manually tested. The system is tested mainly and I say mainly on: - handling the protocol in which the client speaks to the cluster (no need) - handling the protocol in which the cluster speaks to the node (no need) - the query language (when testing the query language we are testing the above 2) - finding any gaps in mistyping a query that can cause a cluster exception of any kind (this has been insanely gone through, I can't even lol) - concurrency with consistency. One test for example is updating multiple documents multiple times through 100 client connections. I don't see the structure of any main method changing. I just want to add tests must be driven meaning.. With a client. You don't test this system like you do a module in GO. You don't just write one test for CountLog(). If I absolutely had to sure. When you test the client>cluster>node(s). So you always do driven testing as a client or multiple clients.

alexpadula 3y ago

Just because you don’t see tests doesn’t mean that there weren’t any before or now. Really not unmanageable at the size of the code base for me. I do agree that method is long but I didn’t want to spread that and a few others across many others it was a design choice. I agree it could be improved but to not want to implement it because of that is your choice. The code is commented and continuously gets more comments and it’s very easy to follow if you read procedurally.

latchkey 3y ago
> for me These keywords are the crux of the issue. What you're expecting is that third parties can read your mind. You have the whole codebase floating in your head so you understand where things are at and how it is laid out. If I'm working with others, or expecting people to pick up my project and either use it or contribute to it, I generally try to make it as easily understood as possible. Common sense is to break the file into manageable sections. Again, tons of ways to skin a cat... but this is a huge red flag, for me.
- alexpadula 3y ago
  Interesting, well understood. I’ll try my best to rethink that logic and get it even shorter. I have no problems with that!! Thank you for taking a look.
- alexpadula 3y ago
  I really tried to make everything as understandable as I could truly and I will continuously strive to do so. Mind you I only built it in around 60 days so everything will come :)
  - latchkey 3y ago
    Please don't get me wrong, this is just hopefully constructive feedback from my personal opinion having written A LOT of mission critical golang code. Again, so many ways to skin a cat... if you can convince other people to contribute to your project as it is, then I'm definitely worthy of ignoring. =)

anonzzzies 3y ago

This is spaghetti to you? You must be lucky as this is very clean to what I get to work with daily (and that’s more like millions LoC.

latchkey 3y ago
What do you want me to say here? It is 3000 lines, in a single file and no tests. I just said "seems like". In order to comprehend it, I'm going to have to parse the whole thing into memory and figure out what it does. Not impossible, but if I'm looking to just fix a specific bug or contribute a new feature, I'd much rather go to the specific file that feature/bug is in, parse a page or two of code on my screen and work from there. It is just how I work. I'm sorry that you have to deal with something that sounds worse. If it was me, I'd spend a lot of extra-curricular time trying to clean up the mess. In return, that would help me get an even better sense of the code.