Encapsulation of functions

[thegreatgunbantoad]

tempScaleLabel updated to use parameters rather than refer to external variables scaleTemperature updated to use parameters rather than refer to external variables

Note change of name of scaleTemperature to scaleTemperatureFromC

[thegreatgunbantoad]

Again not a functional update. Not sure why linting is failing though.

[pppedrillo]

Why?

[thegreatgunbantoad]

Why?

It makes code easier to reuse and understand. Would also allow you to to pull the functions out of the main file and put them in a library. A task it a bit different, like an initialisation task, but functions should generally be properly parameterised.

[pppedrillo]

It makes code easier to reuse and understand. Would also allow you to to pull the functions out of the main file and put them in a library. A task it a bit different, like an initialisation task, but functions should generally be properly parameterised.

"Parameterization" != "to duplicate all the used global/static variables using arguments" And those functions still not in the library or taken out of main file.

Generally why only those two functions were picked? Follow your logic, you'll need to refactor most of the rest of the functions which also use global variables (voltage, tilt etc etc) in the same manner, no? And what if function use 125 global vars? Should it have 125 arguments?

I'd also advise to pass args by reference whenever it makes sense (like here, myData.my_tempscale) and master a switch statement (instead of the if-else-if-else... spaghetti as soon as it is refactoring)

Renaming a function to "FromC" looks good.

[thegreatgunbantoad]

It makes code easier to reuse and understand. Would also allow you to to pull the functions out of the main file and put them in a library. A task it a bit different, like an initialisation task, but functions should generally be properly parameterised.

"Parameterization" != "to duplicate all the used global/static variables using arguments" And those functions still not in the library or taken out of main file.

Generally why only those two functions were picked? Follow your logic, you'll need to refactor most of the rest of the functions which also use global variables (voltage, tilt etc etc) in the same manner, no? And what if function use 125 global vars? Should it have 125 arguments?

I'd also advise to pass args by reference whenever it makes sense (like here, myData.my_tempscale) and master a switch statement (instead of the if-else-if-else... spaghetti as soon as it is refactoring)

Renaming a function to "FromC" looks good.

Only picked those two as it was just a start at getting the functions parametrised. If a function uses lots of global variables I think it might need re-writing. Note the function that writes to the JSON could now just be passed the myData struct. I was really hoping C++ had an iterator like Python that would have made writing to and reading from the JSON slicker.

Yes the plan was to refactor as many functions as is practical. Globals are generally best avoided if reasonable, they can make code very hard to follow.

I sort of prefer pass by value as you know the variable can't change while the function is operating and the function can't alter the inputs. But if you wanted several outputs then pass by ref is handy. Is there a processor advantage to ref over val?

Agree with using a switch.

[pppedrillo]

I was really hoping C++ had an iterator like Python that would have made writing to and reading from the JSON slicker.

There is nothing to do with C++ language, but rather the concrete implementation of JSON (de)/serializer which is not a part of C++ standards.

Yes the plan was to refactor as many functions as is practical.

Thats my main concern. It can be a pull request then all the changes are done, not while work in progress.

Globals are generally best avoided if reasonable, they can make code very hard to follow.

There are some more serious problems with globals and statics, not the only code readability.

I sort of prefer pass by value as you know the variable can't change while the function is operating and the function can't alter the inputs. But if you wanted several outputs then pass by ref is handy. Is there a processor advantage to ref over val?

Passing by val not always guarantees what some data can't be changed. If you pass an object by value and this object has a pointer to data, you still can change a data by this pointer.

Omitting an extra copy is one of advantages. Not a huge deal with one shot functions and PODs, but may cause performance issues if function gets called frequently and you pass a huge object(s) to it.

[PjotrekSE]

I make a struct of my globals, and pass the struct by reference if I need to change a global, and by value if not.

On Sat, 7 Aug 2021, 10:11 pppedrillo, @.***> wrote:

I was really hoping C++ had an iterator like Python that would have made writing to and reading from the JSON slicker.

There is nothing to do with C++ language, but rather the concrete implementation of JSON (de)/serializer which is not a part of C++ standards.

Yes the plan was to refactor as many functions as is practical.

Thats my main concern. It can be a pull request then all the changes are done, not while work in progress.

Globals are generally best avoided if reasonable, they can make code very hard to follow.

There are some more serious problems with globals and statics, not the only code readability.

I sort of prefer pass by value as you know the variable can't change while the function is operating and the function can't alter the inputs. But if you wanted several outputs then pass by ref is handy. Is there a processor advantage to ref over val?

Passing by val not always guarantees what some data can't be changed. If you pass an object by value and this object has a pointer to data, you still can change a data by this pointer.

Omitting an extra copy is one of advantages. Not a huge deal with one shot functions and PODs, but may cause performance issues if function gets called frequently and you pass a huge object(s) to it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/universam1/iSpindel/pull/529#issuecomment-894622929, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIDYCQHMM6RBQDD3VNW7NB3T3TTDLANCNFSM5BJGVLHQ .

[universam1]

@thegreatgunbantoad there are merge conflicts, mind rebasing please?

[thegreatgunbantoad]

@universam1 I'll have look later, not great at the git thing yet, so might need a hand.

[pppedrillo]

@universam1 I'll have look later, not great at the git thing yet, so might need a hand.

I rebased those changes, #543 @universam1

[thegreatgunbantoad]

@pppedrillo does that mean I can just close this request then? Sorry full of a cough and such so my brain is struggling.

[pppedrillo]

@pppedrillo does that mean I can just close this request then? Sorry full of a cough and such so my brain is struggling.

@thegreatgunbantoad Yes, if you are ok with changes from #543. Do get better!

[thegreatgunbantoad]

@pppedrillo frazzled brain thinks looks good.