Subscribe via RSS
Toggle Submenu
  1. Categories
  2. Recent
  3. Tags
View Replies (6)

Resolved Instance Variable

  1. Advice
0
Votes
Undo
  1. Yasir Masood
    Yasir Masood
  2. PowerBuilder
  3. Thursday, 28 April 2022 04:52 AM UTC

Hi,

I have been assigned to maintain/update an older app written PB 10.5. I was able to upgrade the app to PB 2017 without any issue.

The app is a non-visual exe that is triggered through a job after every 30 minutes and is doing some mathematical calculations.

I see that the previous developer(s) used a lot of instance variables. Instead of passing the values as a parameter to the functions(s),

the values are accessed directly from the instance variable.

What would be your suggestion in this situation? Should I change the code and remove the instance variables?

 

Thank

 

PB.INI and ACCESSIBILITY=0
PowerClient app wont run - CORS error
Responses (6)
Andrew Barnes
Andrew Barnes Accepted Answer Pending Moderation
  1. Friday, 29 April 2022 18:32 PM UTC
  2. PowerBuilder
  3. # 1

When it comes to fixing bad code, maybe even completely rewriting it, or just leaving it, I have done all of the above.  My main reason for fixing bad code would be to improve its maintainability, and make it less likely that a subsequent programmer will get confused by the code when making a change.  And if it is a piece of code that I will likely modifying semi-regularly, I might want to change it so that subsequent programming on it is easier for me.

If you do to fix route, make certain that you do your due diligence and analyze the code well enough to be certain that you do not break something.  Are the instance variable private?  if not, you do have to search the entire application to see that there is no other coded referencing them for example.

Comment
Brad Mettee
  1. Brad Mettee
  2. Saturday, 30 April 2022 14:52 PM UTC
You can also do diagnostics by making them all private and seeing where else code errors pop up when you do a test compile.
  1. Helpful
There are no comments made yet.
Benjamin Gaesslein
Benjamin Gaesslein Accepted Answer Pending Moderation
  1. Friday, 29 April 2022 11:33 AM UTC
  2. PowerBuilder
  3. # 2

Imho, adhering to the "if it ain't broke, don't fix it" dogma is the main reason crappy legacy codebases exist. I would like to update that to "if it ain't broke but smells awful, improve it!" Spend some time to implement unit tests to make sure that refactoring won't break it. Repeat.

Comment
mike S
  1. mike S
  2. Friday, 29 April 2022 13:46 PM UTC
agreed, my "aint' broke" is a somewhat flip response.

is it worth the time and money? why do you get back from the refactoring? is it worth the risk of breaking something? it's always a case-by-case basis as @kevin said.



i've spent a lot of time and money on refactoring my pb application. some was not worth it, while some paid for itself (and continues to pay for itself) 1000x over.
  1. Helpful 1
There are no comments made yet.
Kevin Ridley
Kevin Ridley Accepted Answer Pending Moderation
  1. Thursday, 28 April 2022 22:07 PM UTC
  2. PowerBuilder
  3. # 3

I would say "it depends".  It's always a difficult decision to take working code and refactor it.  Do the instance variables have values that can change during 1 cycle of the app?  Either way you can always setup some sort of of_reset() function to clear the values and get them set again.  Depending on the usage, I usually like to keep my functions stateless were possible.  Some business cases require some state being saved, so like I said, it depends.

 

Kevin

Comment
There are no comments made yet.
John Fauss
John Fauss Accepted Answer Pending Moderation
  1. Thursday, 28 April 2022 14:16 PM UTC
  2. PowerBuilder
  3. # 4

Greetings, Yasir -

A related factor to consider: Are the instance variables used to pass data values both into and out of methods?

I agree this is an example of poor programming practices, but I would not change it unless there was a very good reason to do so, particularly if this is code that is currently running or is part of a production process. The old adage "If it ain't broke, don't fix it" applies in this case, I believe. My two cents, FWIW.

Best regards, John

Comment
Yasir Masood
  1. Yasir Masood
  2. Thursday, 28 April 2022 14:40 PM UTC
Instance Variables are set at the start of the app (constructor event). Then these variables are accessed in different functions. There is no modification to variables.
  1. Helpful
There are no comments made yet.
Yasir Masood
Yasir Masood Accepted Answer Pending Moderation
  1. Thursday, 28 April 2022 14:15 PM UTC
  2. PowerBuilder
  3. # 5

It is a non visual object so there is no user interaction

Comment
There are no comments made yet.
mike S
mike S Accepted Answer Pending Moderation
  1. Thursday, 28 April 2022 14:10 PM UTC
  2. PowerBuilder
  3. # 6

if it aint broke don't fix it

Comment
Yasir Masood
  1. Yasir Masood
  2. Thursday, 28 April 2022 14:15 PM UTC
Thanks
  1. Helpful 1
There are no comments made yet.
  • Page :
  • 1


There are no replies made for this question yet.
However, you are not allowed to reply to this question.