Win a copy of TDD for a Shopping Website LiveProject this week in the Testing forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Paul Clapham
  • Ron McLeod
  • Jeanne Boyarsky
  • Tim Cooke
Sheriffs:
  • Liutauras Vilda
  • paul wheaton
  • Henry Wong
Saloon Keepers:
  • Tim Moores
  • Tim Holloway
  • Stephan van Hulst
  • Carey Brown
  • Frits Walraven
Bartenders:
  • Piet Souris
  • Himai Minh

Help me make this method re-usable

 
Ranch Hand
Posts: 80
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I have a method that i need to improve. Basically, i am using this code to create an arraylist for a JSF page. I have a ton of classes that i need to build arraylists from... All these classes just have name and id attributes. How could i make this method so that i can reuse it for all these simple classes. What topics should i be researching.

is this codeblock not a good candidate to build out and should i just be reusing it..



same as

 
Ranch Hand
Posts: 74
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
As far as I can see, your Depth and TopType are some sort of "product properties". As you said they both (and all the other tons of similar classes) have an 'id' and 'name' property. This kind of "screams" for a redesign of your Product class
instead of having getters for depthId, depthName, topTypeId, topTypeName etc., you should hold Depth and TopType beans inside your Product:

Once you have this, you need to make sure that Depth, TopType etc. all extend from a common abstract class (let's say ProductProperty) that simply holds id and name fields (in case the ID type is not always the same -- let's say Long -- you should choose a type that fits all: Serializable, Number or whatever) and provides getters/setters for those.

Your ProductProperty subclasses (Depth, TopType, etc.) will need to overwrite java.lang.Object.equals() and java.lang.Object.hashCode() and also they should implement Comparable since there seems to be some sort of natural order for those classes (you can make the abstract class ProductProperty implement Comparable itself with some generic tricks, so that you won't need to worry about making every subclass Comparable).

Once you have this sorted out, you should be able to replace buildArrayListOfAllDepths() and buildArrayListOfAllTopTypes() with a common buildList() method declared in some parent class.
This parent class (let's call it ListBuilder) will be generic and will also need to be able to fetch the property it handles (Depth or whatever) from a product:



And in the class that handle depths, you will only have:


Just a tiny note: although in this example I tried to keep the code as you posted it, I would suggest that you don't sort the same elements twice.


If you really want to use a comparator (cannot rely on any natural order or something), you will need to provide the comparator with one more abstract method returning Comparator<T> that will be implemented in specific subclasses (a Comparator<Depth> will be returned by the "DepthListBuilder" subclass).


Hope this helps.

 
Christopher Whu
Ranch Hand
Posts: 80
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
That is exactly what i was hoping to see... Ty very much...

 
Christopher Whu
Ranch Hand
Posts: 80
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Is there a good rule or guideline for when something deserves it's own class?
 
Sheriff
Posts: 22645
124
Eclipse IDE Spring VI Editor Chrome Java Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Christopher Whu wrote:Ty


Sorry to nitpick, but please UseRealWords: "thank you".
 
Bartender
Posts: 4179
22
IntelliJ IDE Python Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Not to confuse things too much - Costi's response is really good. I agree that there probably should be re-factoring involved for the best result. But there could be reasons why this isn't possible, and in those cases there are alternatives.

For example, your 'parameters' have an ID and a name. What happens when the ID for different parameters is not of the same type? Then having a single parent type may not be useful, or the inheritance tree may get too complicated. So an alternative for this type of refactoring would be a type of injection where you provide an Object which knows all the little bits that are unique to each parameter while the original method knows how to call the correct methods in order to get what it needs. Here is an example:

Your original code was this:


Let's remove the pieces that are specific to Depth, and move them to a mythical 'ListAdapter':


You can see that the adapter is responsible for creating new parameter instances, retrieving ids, lists, and comparators, and checking if it is editable. From this, we can make an interface. At his point we have to add some reality checks - for example, where would the information each method requires come from:

Note that 'ProductUtilities' is the class with the 'buildListOfProducts()' method. It also has the 'getOrigDepthArrayList(),' 'checkDepthEditable(),' and related methods.

We can then make implementations of the ListAdapter interface for each type of parameter you want to work with. In this case I am using a Factory to generate the instances I need as well, just to make them more accessible:

These implementations are little more than wrappers around the code you used in the buildArrayListOfAllDepths() method, and most of them just delegate to some other code you already have (for example the getOrigDepthArrayList() method).

Your 'buildArrayListOfxxxx()' methods then turn to this:

With no changes to the rest of your code.

The difference between this approach and the one already presented by Costi:
1) This doesn't force a relationship between TopType and Depth (such as a common super-type) if that relationship doesn't make sense. On the other hand, if the relationship does make sense you would be better of making the correct inheritance tree which makes that relationship explicit.
2) This approach allows for a bit more flexibility in that the types of the IDs and Names do not need to be related between the parameters. It also allows for less 'relationship' in other ways as well (for example, if you needed to retrieve lists from different places, call un-related methods, or methods with different types and counts of parameters, etc...)
3) This approach requires less change of your existing code. You refactor the 'buildArrayListOfxxx()' methods and add the generic version. But you do not need to change the TopType or Depth classes, or how / where the Lists are stored and how they are accessed.
4) Costi's approach will lend itself to similar optimizations in other parts of code, make those changes more obvious and easier to make. The approach here is isolated, and would need to be re-done for each piece of code you want to re-factor. Once done it does not make it easier to do elsewhere.
5) My approach is filled with interface dependencies. For example the adapter needs to know the names of the methods in the 'ProductUtilities' class for retrieving lists. Those could change, and that change would require fixes in two places. As a matter of fact those methods might be best left private.

Which one you choose depends on your situation. If you have time and the relationships make sense, Costi's method of refactoring will probably be better and lend your code to better optimizations in other areas. If you have a large system and you have to worry about how the changes you make here affect how other classes/methods interact then the method presented above will probably minimize these interactions.
 
Christopher Whu
Ranch Hand
Posts: 80
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Another great response... Again thanks... I am completely self taught and seeing multiple ways of improving the code is going to open my mind to alot of new concepts... I am very grateful...
 
Costi Ciudatu
Ranch Hand
Posts: 74
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Is there a good rule or guideline for when something deserves it's own class?


In this particular case, you seem to have already "smelled" that Depth/TopType/etc. do deserve their own classes (since you did create those classes, didn't you ?). So you seem to already know "when something deserves its own class".

But if you want some "rule of thumb" (not something by the book, just something I try to keep in mind when I'm desigining my class hierarchies), start from describing your business problem in natural language (for instance, describing the problem you're facing with to someone that doesn't know anything about programming): if you find yourself saying that each product has a particular "depth" and a particular "top type", these are very likely to become classes (as abstract data types) and also fields of your product (as objects / instances of those types).

On the other hand, if you'll try to explain that a product in your use case does hold a depthID (which is some number with no real-life significance) you'll probably notice some confusion in your audience feedback; and you'll need to rephrase the "story" so that they'll understand what you mean: you'll talk about Depth and don't mention the (artificial) depthID.

In other words, I would say that one of the main (conceptual) advantages of OOP is that it's a paradigm can follow the "story" quite closely. All you need to do is to write your code so that it "sticks to the story" and make sure your class hierarchy "tells the original story" as adequately as possible. By the way, I don't really know what your "story" is for this particular topic (I was mostly guessing in my reply above), but Steve has provided you with a really good solution that is not based on any guessing.

However, especially since you're self taught, I think a good advice regarding how to spot classes in your business problem or specification would be to just underline all the nouns that you're using when you're describing your business situation to some non-technical person; those are most likely your classes; the verbs will be sometimes relations between your classes and sometimes methods. (Of course, this is over-simplified, but this is what "rules of thumb" are all about, isn't it ?)
 
Costi Ciudatu
Ranch Hand
Posts: 74
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
P.S.:

In case you'll find the solution that I posted useful, keep in mind that my "ListBuilder" declaration assumes that you
declared your "ProductProperty" class to implement Comparable itself (otherwise, Collections.sort() will complain).

In order to do this, I already mentioned you need to use a generics "trick":


And the subclasses will look like this:


Not very nice, but this is the only way (that I know of, at least) you can make the superclass comparable and still
enforce the subclasses to only be comparable to their own subtype only.

If you will choose to declare every subclass as Comparable (and not make the ProductProperty Comparable, perhaps because
this does not make sense for all its subclasses), you'll need to declare the "ListBuilder" type parameter like this:

If you'll use comparators, this won't be a problem at all.
 
This tiny ad will self destruct in five seconds.
free, earth-friendly heat - a kickstarter for putting coin in your pocket while saving the earth
https://coderanch.com/t/751654/free-earth-friendly-heat-kickstarter
reply
    Bookmark Topic Watch Topic
  • New Topic