Learn More
Resend Email
28
txCodeWizardry
7y

Code review time:

Hey Rudy, can you approve my PR? ??? Shouldn't it be can you review my PR?(thought to myself)
Anyway, as a new practice, we(royal we) do not approve PRs with js files. If we touch one, we convert it to typescript as part of a ramp up to a migration that never seems to get here. But I digress.
I look at the laziest conversion in history.

Looked like
Import 'something';
Import 'somethingElse';
Import 'anotherSomething';

export class SomeClass {
public prop1: any;
public prop2: any;
public prop3: any;

public doWork(param: any){
let someValue = param;
// you get the idea
return someValue;
}
}

Anyway, I question if all the properties need to be visible outside of the class since everything was public.
Then if the dev could go and use type safety.
Then asked why not define the return type for the method since it would make it easier for others to consume.
Since parts of the app are still in js, I asked that they check that that the value passed in was valid(no compilation error, obviously).
Also to use = () => {} to make sure "this" is really this.
I also pointed out the import problem, but anyway.

I then see the his team leader approve the PR and then tell me that I'm being too hard on his devs. ????

Do we need to finish every PR comment with "pretty please" now?
These are grown men and women, and yet, it feels oddly like kindergarten.

I've written code in the past that wasn't pretty and I received "WTF?" as a PR comment. I then realized I ate sh*t on that line of code, corrected it and pushed the code. Then we went to Starbucks.
I'm not that old(35), but these young devs need to learn that COMPILERS DO NOT CARE ABOUT YOUR FEELINGS!!!!!
Ahhhhhh
Much better.
Thanks for the platform.

undefined
Favorite
Ranter
Comments
  • 6
    7y
    Agreed. Compilers don't care about your feelings.
  • 6
    7y
    DevRant T-shirt Idea #727

    Compliers don't care about your feelings
  • 0
    7y
    Where did you want him to use arrow functions? All I see is a class with one method, did you suggest him to make the method an arrow function?
    Like this.blah = () => {}?
  • 1
    7y
    @Froot - it's a simpler way to make sure "this" is still referring to the instance of the class.
  • 1
    7y
    @Froot - also, you wouldn't use this on the method declaration. The arrow function is only different at declaration.
    But once it's called, it would be this.method();
    Or from outside, this.instanceOfThatClass.method(); passing in whatever parameters the signature requires.
  • 0
    7y
    @txCodeWizardry Nooooooooo... Wait your serious? Your really suggesting creating all methods of a class inside the constructor as arrow functions? So you have a class that's empty apart from one huge ass 1000 line constructor?

    Man that's some bad code style beyond repair
  • 1
    7y
    Wait, you seriously believe that that is even remotely what I suggested?
  • 1
    7y
    Class with some properties and one or more methods

    At declaration(where you define them) you give them whatever access to the outside world(private, protected, public)

    When you declare the method, you use that, not at implementation. I have no idea how you arrived at your conclusion of what I explained.

    No shit you don't declare in the constructor.

    Constructors are where you construct the object based on either default values or params passed in when the class gets instantiated.

    Simple oop concepts.
devRant © 2021 Hexical Labs LLC
Privacy Policy  |  Terms of Service
Add Comment