search

DavidFeldhoff / al-codeactions

MIT License
17 stars 8 forks source link

Make a variable global: is this really good? #142

Closed JalmarazMartn closed 1 year ago

JalmarazMartn commented 1 year ago

Good evening, I love your extension, but i Don´t like this action, "Make a variable global". I realized just today that this exists. In a world when people tell all the time to avoid global variables, "be as local as you can", "avoid side efects", is necesary or simply good to offer turn a local variable in a global one?

In my opinion this ease a bad practice.

Best regards and thanks for your extension.

DavidFeldhoff commented 1 year ago

Good evening and thanks for the feedback! I agree with you that code should be as local as possible. Especially if the variables are stateful! That's always a big warning for me.

But if it's a stateless CodeUnit, I don't see why that shouldn't be global. Let's take the "Library - Assert" as example. Why should I create that in each of my test procedures? That doesn't make sense. I'm just creating multiple instances of a CodeUnit which consumes memory which is stateless anyway.

That's at least my viewing point. I'm with you that it shouldn't be used all the time. If a developer makes generally everything public - that's what code reviews are for ;) And of course if you don't like it, you don't have to use it. But I won't remove the possibility due to the reasons I explained above. I hope you can follow my explanation.

Wish you a great evening and all the best David

DavidFeldhoff commented 1 year ago

The only thing I could do for you would be to add a setting that turns off the code action, so that you could set it in your workspace files and therefore deactivate it for your team colleagues as well, but as I said: there are reasons that variables can still be global and your colleagues could move the local variable manually as well, so it wouldn't make a difference in my eyes.

JalmarazMartn commented 1 year ago

David, thank you for your response. Close the issue if you want. You feel that is necessary from your experience, and is all that actually matters. I only wanted to know the reason for this action code.

About test code, partially agree, because in my test codeunits I declare libraries as global too. But I think it is a very especifique example, an exception.

Also I think that it is very curious that we consider an object stateless and safe to declare as global... if it hasn't global variables inside.

Forgive me, I love these coding discussions and don`t want to steal your time. Have a nice weekend!

El mié., 7 de septiembre de 2022 18:52, David Feldhoff < @.***> escribió:

The only thing I could do for you would be to add a setting that turns off the code action, so that you could set it in your workspace files and therefore deactivate it for your team colleagues as well, but as I said: there are reasons that variables can still be global and your colleagues could move the local variable manually as well, so it wouldn't make a difference in my eyes.

— Reply to this email directly, view it on GitHub https://github.com/DavidFeldhoff/al-codeactions/issues/142#issuecomment-1239644473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKQV3T3ZTFFXCGLXKUYEUMLV5DB4XANCNFSM6AAAAAAQG2SS64 . You are receiving this because you authored the thread.Message ID: @.***>