Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link
  1. Not only is it bad practice, I'd say it's horrible practice. Save yourself, or others which will be dealing with your code, some future headache and use as few static variables as possible, preferably none! (The reason why I say this is horrible could have something to do with the code I have to deal with at work, anyways, it is still not good practice...)

  2. I'd say that not using the default constructor is perfectly fine, if all your Reporter objects needs a jsonFilePath then it's good to pass it in the constructor. Also, if this value shouldn't change then you can make it readonly.

  3. If by disposing you mean "clearing it's state" or something, then no you don't need to. Garbage collector will take care of that. However, if you keep using the static variables then you might run into problems if you want to create another one.

#So, how to get rid of these static variables?

So, how to get rid of these static variables?

It's quite easy actually: Tell, don't ask.

You haven't given much code here so I'll have to try to explain how it generally works:

In your json class (which I will call Foo from now on since I don't know it's actual name), or whatever you have, instead of using Reporter.ShouldNotify, create an instance variable of the Reporter type. Then if you call/create your Foo class from your Reporter class, you tell the Foo object about it's Reporter. For example:

// In your reporter class
Foo foo = new Foo();
foo.setReporter(this); // sorry for the Java naming conventions here, but I hope you get the idea

In Foo you can then use your Reporter object which you retreived from the setReporter method to use that particular object.

Note that one can take the "Tell, don't ask" thing even further, but with the code in your question this is what I think you need for now.

  1. Not only is it bad practice, I'd say it's horrible practice. Save yourself, or others which will be dealing with your code, some future headache and use as few static variables as possible, preferably none! (The reason why I say this is horrible could have something to do with the code I have to deal with at work, anyways, it is still not good practice...)

  2. I'd say that not using the default constructor is perfectly fine, if all your Reporter objects needs a jsonFilePath then it's good to pass it in the constructor. Also, if this value shouldn't change then you can make it readonly.

  3. If by disposing you mean "clearing it's state" or something, then no you don't need to. Garbage collector will take care of that. However, if you keep using the static variables then you might run into problems if you want to create another one.

#So, how to get rid of these static variables?

It's quite easy actually: Tell, don't ask.

You haven't given much code here so I'll have to try to explain how it generally works:

In your json class (which I will call Foo from now on since I don't know it's actual name), or whatever you have, instead of using Reporter.ShouldNotify, create an instance variable of the Reporter type. Then if you call/create your Foo class from your Reporter class, you tell the Foo object about it's Reporter. For example:

// In your reporter class
Foo foo = new Foo();
foo.setReporter(this); // sorry for the Java naming conventions here, but I hope you get the idea

In Foo you can then use your Reporter object which you retreived from the setReporter method to use that particular object.

Note that one can take the "Tell, don't ask" thing even further, but with the code in your question this is what I think you need for now.

  1. Not only is it bad practice, I'd say it's horrible practice. Save yourself, or others which will be dealing with your code, some future headache and use as few static variables as possible, preferably none! (The reason why I say this is horrible could have something to do with the code I have to deal with at work, anyways, it is still not good practice...)

  2. I'd say that not using the default constructor is perfectly fine, if all your Reporter objects needs a jsonFilePath then it's good to pass it in the constructor. Also, if this value shouldn't change then you can make it readonly.

  3. If by disposing you mean "clearing it's state" or something, then no you don't need to. Garbage collector will take care of that. However, if you keep using the static variables then you might run into problems if you want to create another one.

So, how to get rid of these static variables?

It's quite easy actually: Tell, don't ask.

You haven't given much code here so I'll have to try to explain how it generally works:

In your json class (which I will call Foo from now on since I don't know it's actual name), or whatever you have, instead of using Reporter.ShouldNotify, create an instance variable of the Reporter type. Then if you call/create your Foo class from your Reporter class, you tell the Foo object about it's Reporter. For example:

// In your reporter class
Foo foo = new Foo();
foo.setReporter(this); // sorry for the Java naming conventions here, but I hope you get the idea

In Foo you can then use your Reporter object which you retreived from the setReporter method to use that particular object.

Note that one can take the "Tell, don't ask" thing even further, but with the code in your question this is what I think you need for now.

Source Link
Simon Forsberg
  • 59.7k
  • 9
  • 157
  • 311
  1. Not only is it bad practice, I'd say it's horrible practice. Save yourself, or others which will be dealing with your code, some future headache and use as few static variables as possible, preferably none! (The reason why I say this is horrible could have something to do with the code I have to deal with at work, anyways, it is still not good practice...)

  2. I'd say that not using the default constructor is perfectly fine, if all your Reporter objects needs a jsonFilePath then it's good to pass it in the constructor. Also, if this value shouldn't change then you can make it readonly.

  3. If by disposing you mean "clearing it's state" or something, then no you don't need to. Garbage collector will take care of that. However, if you keep using the static variables then you might run into problems if you want to create another one.

#So, how to get rid of these static variables?

It's quite easy actually: Tell, don't ask.

You haven't given much code here so I'll have to try to explain how it generally works:

In your json class (which I will call Foo from now on since I don't know it's actual name), or whatever you have, instead of using Reporter.ShouldNotify, create an instance variable of the Reporter type. Then if you call/create your Foo class from your Reporter class, you tell the Foo object about it's Reporter. For example:

// In your reporter class
Foo foo = new Foo();
foo.setReporter(this); // sorry for the Java naming conventions here, but I hope you get the idea

In Foo you can then use your Reporter object which you retreived from the setReporter method to use that particular object.

Note that one can take the "Tell, don't ask" thing even further, but with the code in your question this is what I think you need for now.

lang-cs

AltStyle によって変換されたページ (->オリジナル) /