This week's book giveaway is in the OO, Patterns, UML and Refactoring forum. We're giving away four copies of Refactoring for Software Design Smells: Managing Technical Debt and have Girish Suryanarayana, Ganesh Samarthyam & Tushar Sharma on-line! See this thread for details.
I have a class that does some processing and creates an ArrayList.
The List is added to at various levels like this.
I feel that this is not the right way to implement this since this code is not easy on somebody looking at this for the first time. You know that method y updates the list and that method z does not update the list only by going to those methods.
I feel the better alternative would be:
or maybe this:
Please share your thoughts. What is the "best practise" here?
You can only use "public" "protected" "private" or "static" outside methods, so your code won't compile.
Joined: Oct 13, 2005
Start by giving your methods proper names.
A method should do one thing and one thing only, as far as possible. Your methods seem to be doing several things and it is not obvious to the user what the methods do.
Joined: Oct 08, 2002
Sorry, the sample code was meant only to demonstrate the different ways of implementing the same thing. I am adding the edited code below.
In #1, I have defined an instance variable named list. This is updated by myMainMethod and by addToListIfEvenNumber. By looking at myMainMethod, you cant tell if the list is getting updated in addToListIfEvenNumber and in justSomeMoreProcessing. So I would say this code is not very readable.
In #2, I have defined a method variable named list. By looking at myMainMethod, you can tell if the list is likely to get updated in addToListIfEvenNumber. justSomeMoreProcessing will not be updating the list since it has no access to the list. Better readability but goes against the best practice of not updating an argument .
Implementation #3, seems a round about way of achieving what was done in step 2.
I recommend #2, but is it the best? The actual code that I am working on is a lot more complex than this but I think the example sums up my concern. Please ignore any compilation issues.
I would agree with you, #2 is the best approach. When a method is supposed to work on a list, it should receive the list as parameter. Way #1 can be useful if you have objects that most of your methods operate on, but in general you should avoid global variables where possible.
Joined: Oct 13, 2005
1 and 2 are as different as chalk and cheese. There is a world of difference between using a field and using a local variable; remember local variables vanish never to be seen again when their method completes.
3 does something similar no 2. But no 3 potentially makes another List available for other methods to use. I trust you don't use raw type Lists in "real life".
I think you need to try them out with some statements to print the contents of the different Lists; then you can see what you are doing. Then write some proper documentation comments for every method, then compile with the javadoc tool and see whether you can understand from the HTML files what the methods do.
I’ve looked at a lot of different solutions, and in my humble opinion Aspose is the way to go. Here’s the link: http://aspose.com