Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Junilu Lacar wrote:I've already warned you about having lots of statics in your class before.
Junilu Lacar wrote:Do you know about enum types? Your weapon types would be better defined as an enum type than as static fields in the Weapon class.
Junilu Lacar wrote:Also, it's not a good idea to mix infrastructure concerns like code that deals with data file IO in a domain class like Weapon. That makes your design and code more brittle.
I have been reading on the builder since I read your post and will be fixing up all 7 classes I did similar to this one, thanks again JuniliJunilu Lacar wrote:Any more than that, you should consider using a builder.
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Campbell Ritchie wrote:
Never write an empty catch block. If you have an exception in the deserialising method, why are you trying to catch it rather than letting it propagate?
Campbell Ritchie wrote:Formatting: Start by getting rid of the //===== comments. Separate logical parts of the program by single empty lines. Separate successive methods by single empty lines. Also break the long lines into several lines: read this. Also look at the old Sun style guide, which whould advise you how to shorten your lines.
Don't try to write a method all on one line. A method with a single statement should occupy at least three lines. Don't write a loop or if‑else without {} and without indentation.
Campbell Ritchie wrote:Why have you got public fields, particularly when some point to mutable reference types?
Campbell Ritchie wrote:Why have you got those static methods?
Campbell Ritchie wrote:There is something wrong about the addAttachs method. Apart from the fact that attachs is unpronounceable in English, that method appears to add the weapon to the attachment. That looks the wrong way round. Also, why are you rewriting the file so often?
Again no idea why I did that, seems like something else I need to fixCampbell Ritchie wrote:Never write an empty catch block. If you have an exception in the deserialising method, why are you trying to catch it rather than letting it propagate?
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Yosuf Ibrahim wrote:
Junilu Lacar wrote:I've already warned you about having lots of statics in your class before.
Sorry, I thought You did not like a class containing statics only and I popped the question what do I do then with classes whose methods will only be used once but got no reply yet.
All things are lawful, but not all things are profitable.
Yosuf Ibrahim wrote:
Campbell Ritchie wrote:Why have you got those static methods?
Because I am dealing with other objects of this class, not the one created in this class, is there any other way to extract those objects out of the files without using static methods??
All things are lawful, but not all things are profitable.
All things are lawful, but not all things are profitable.
Knute Snortum wrote:
Maybe you can give us an illustration with code of what you're trying to accomplish with static members.
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Yosuf Ibrahim wrote:
Knute Snortum wrote:
Maybe you can give us an illustration with code of what you're trying to accomplish with static members.
I am trying to extract a list of objects of the type class from files stored in a directory. Therefore I used static methods to that. The methods are at the bottom half of my pasted code up top in the first post.
All things are lawful, but not all things are profitable.
What makes you think that static fields, as shown in lines 30‑40, should be public? Particularly since two fields point to mutable reference types.Yosuf Ibrahim wrote:. . . Apart from the static ones, seems like all my field are private, or am I missing something? . . .
The deserialise method should be static because it has to be called before any instances exist. But what about the others? Why are you using the existence of a file to show the existence of a weapon?Campbell Ritchie wrote:Why have you got those static methods?
Because I am dealing with other objects of this class, not the one created in this class, is there any other way to extract those objects out of the files without using static methods??
That's a pleasure. . . Thanks, Campbell appreciate your time
Campbell Ritchie wrote:What makes you think that static fields, as shown in lines 30‑40, should be public? Particularly since two fields point to mutable reference types.
Campbell Ritchie wrote:Why are you using the existence of a file to show the existence of a weapon?
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Knute Snortum wrote:...you don't have to use static methods for that...
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Yosuf Ibrahim wrote:public static final String[] WEAPON_TYPES_LIST ...
public static final String[] WEAPON_TYPES_SEARCH_LIST ...
Liutauras Vilda wrote:
...are you familiar with Streams? Most of your methods look to me perfect candidates for consice substitutions with streams.
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Carey Brown wrote:Interesting project you have here. Thanks for sharing it with us.
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Yosuf Ibrahim wrote:
Knute Snortum wrote:...you don't have to use static methods for that...
You tell me I don't have to use static methods but proceed by telling me to create a class and in your example its all static methods only, I am a bit lost now.
All things are lawful, but not all things are profitable.
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
Consider Paul's rocket mass heater. |