Design question - methods calling methods
in [Java Programming]
|
From: Rhino on 22 May 2010 16:30 Lew <noone(a)lewscanon.com> wrote in news:ht76kf$e1c$1(a)news.albasani.net: > Rhino wrote: >> ... getLocales(), which returns a TreeMap, and then displays the >> contents of the TreeMap on the console. > > It's mostly a good idea to declare variables as an interface type > rather than a concrete type. The rule of thumb is that you declare > the variable with the loosest possible type that has the behavioral > contract you need. > > Thus, you probably want 'getLocales()' to return a 'SortedMap', as > someone suggested in another thread, unless there's something specific > about 'TreeMap' in particular that requires you use only that type or > a subtype thereof. > For a second there, I had no idea what you were proposing. But I reviewed the Collection Classes in the Java Tutorial and I think I get it now. In all honesty, I hadn't even thought of SortedMap when I wrote the code again the other day. I just knew that I wanted the result to be in alphabetical order, not random the way that Locales.getAvailableLocales() provides them. The article on the Map interface pointed out that TreeMap would assure that I had alphabetical order so I went with that. Now that you've reminded me about SortedMap, I can see the merit of it. It's not much different than TreeMap but it does give those additional features, like range operations. I don't see those as being _necessary_ for my humble little getLocales() method, which I'm really just writing for myself, but some future user of the class could conceivably benefit from those extra features. Or maybe _I_ will get a benefit from those features a little further down the road! I've modified the code to produced a SortedMap - just replaced all "Map" with "SortedMap", dead easy! - and reran my unit tests. Naturally, they still worked fine. Hmm. Did I do this right? I had this: ========================================================================= public Map<String, Locale> getLocales() { Map<String, Locale> sortedLocales = new TreeMap<String, Locale>(); for (Locale oneLocale : Locale.getAvailableLocales()) { sortedLocales.put(oneLocale.getDisplayName(locale), oneLocale); } return sortedLocales; } ======================================================================== and changed it to: ======================================================================== public SortedMap<String, Locale> getLocales() { SortedMap<String, Locale> sortedLocales = new TreeMap<String, Locale>(); for (Locale oneLocale : Locale.getAvailableLocales()) { sortedLocales.put(oneLocale.getDisplayName(locale), oneLocale); } return sortedLocales; } ======================================================================== The first line of the revised method looks a bit funny: SortedMap ... = TreeMap .... Did I do what you meant? Sigh! I still struggle with understanding what I am doing sometimes.... I still don't REALLY understand the difference between these: - Map<String, Locale> sortedLocales = new TreeMap<String, Locale>(); - SortedMap<String, Locale> sortedLocales = new TreeMap<String, Locale> (); - Map<String, Locale> sortedLocales = new SortedMap<String, Locale>(); - and so forth Or, by the same token: - Calendar cal = Calendar.getInstance(); - Calendar cal = new GregorianCalendar(); - GregorianCalendar gcal = new GregorianCalendar(); If I had to write an exam, clearly articulating the distinction between those and what the implications of each is, I'd surely make a hash of it.... I can tell if I'm getting a compile error and I can tell if my code is doing what I want it to do but I still don't always understand WHY one thing is better than an other or what the real difference is between things that look very very similar..... That doesn't fill me with optimism about my ability to persuade an employer that I would be a useful addition to their team. I imagine they're going to want people that know that stuff backwards and forwards.... -- Rhino
From: Roedy Green on 22 May 2010 17:18 On Fri, 21 May 2010 20:20:05 +0000 (UTC), Rhino <no.offline.contact.please(a)example.com> wrote, quoted or indirectly quoted someone who said : >Is it bad design for any reason for methods in one class to call other >methods in the same class to do some of their work for them? I'm pretty >sure the answer is no but this is just a sanity check to make sure I'm not >forgetting an important factor. As a general rule, avoid ever cloning code, unless it is just to get a first cut before you brutally rehash it. You don't want snippets of code repeated in your project. Sooner or later you will want to change them, and it is impossible to ensure all are kept in sync. If later you decide some of the clients need a variant and others do not, it is lot easier to find all the occurrences if they have been encapsulated. The smaller your methods and the greater the reuse, the easier it is to maintain code. You will only have to change one method. This is the great truth you learn coding in FORTH. -- Roedy Green Canadian Mind Products http://mindprod.com Beauty is our business. ~ Edsger Wybe Dijkstra (born: 1930-05-11 died: 2002-08-06 at age: 72) Referring to computer science.
From: Jeff Higgins on 22 May 2010 17:18 On 5/21/2010 4:20 PM, Rhino wrote: > The situation that inspired the question is getLocales() method I've been > discussing in another thread. A recent post to this group prompts me to post these links: <http://developers.sun.com/global/> <http://developers.sun.com/global/technology/arch/>
From: Tom Anderson on 22 May 2010 17:31 On Sat, 22 May 2010, Rhino wrote: > Lew <noone(a)lewscanon.com> wrote in news:ht76kf$e1c$1(a)news.albasani.net: > >> Rhino wrote: >>> ... getLocales(), which returns a TreeMap, and then displays the >>> contents of the TreeMap on the console. >> >> It's mostly a good idea to declare variables as an interface type >> rather than a concrete type. Rhino, read that sentence again. >> The rule of thumb is that you declare the variable with the loosest >> possible type that has the behavioral contract you need. And that one. And again. Now proceed with the rest of this post. >> Thus, you probably want 'getLocales()' to return a 'SortedMap', as >> someone suggested in another thread, unless there's something specific >> about 'TreeMap' in particular that requires you use only that type or >> a subtype thereof. > > For a second there, I had no idea what you were proposing. > > But I reviewed the Collection Classes in the Java Tutorial and I think I > get it now. > > In all honesty, I hadn't even thought of SortedMap when I wrote the code > again the other day. I just knew that I wanted the result to be in > alphabetical order, not random the way that Locales.getAvailableLocales() > provides them. You make that desire concrete by the type of the object holding the locales - you want them to be sorted, so it's a SortedMap. > The article on the Map interface pointed out that TreeMap would assure > that I had alphabetical order so I went with that. Now that you've > reminded me about SortedMap, I can see the merit of it. It's not much > different than TreeMap but it does give those additional features, like > range operations. I don't see those as being _necessary_ for my humble > little getLocales() method, which I'm really just writing for myself, > but some future user of the class could conceivably benefit from those > extra features. Or maybe _I_ will get a benefit from those features a > little further down the road! You're on the wrong track here. The extra features are not what SortedMap is about - it's fundamentally about that first paragraph in its javadoc: A Map that further provides a total ordering on its keys. The map is ordered according to the natural ordering of its keys [...] This order is reflected when iterating over the sorted map's collection views (returned by the entrySet, keySet and values methods). Several additional operations are provided to take advantage of the ordering. Yes, there are several additional operations. But the heart of the matter is that the map has an order, which governs iteration over its contents. > I've modified the code to produced a SortedMap - just replaced all "Map" > with "SortedMap", dead easy! - and reran my unit tests. Naturally, they > still worked fine. > > Hmm. Did I do this right? > > I had this: > > ========================================================================= > public Map<String, Locale> getLocales() { > > Map<String, Locale> sortedLocales = new TreeMap<String, Locale>(); > for (Locale oneLocale : Locale.getAvailableLocales()) { > sortedLocales.put(oneLocale.getDisplayName(locale), oneLocale); > } > > return sortedLocales; > } > ======================================================================== > > and changed it to: > > ======================================================================== > public SortedMap<String, Locale> getLocales() { > > SortedMap<String, Locale> sortedLocales = new TreeMap<String, Locale>(); > for (Locale oneLocale : Locale.getAvailableLocales()) { > sortedLocales.put(oneLocale.getDisplayName(locale), oneLocale); > } > return sortedLocales; > } > ======================================================================== Spot on. > The first line of the revised method looks a bit funny: > SortedMap ... = TreeMap .... > > Did I do what you meant? Yes. You did *exactly* what Lew meant when he said: It's mostly a good idea to declare variables as an interface type rather than a concrete type. SortedMap is an interface type - it says 'this map is sorted somehow'. TreeMap is a concrete type - it says 'this map is implemented as a red-black tree, which incidentally results in it being sorted'. > Sigh! I still struggle with understanding what I am doing sometimes.... I > still don't REALLY understand the difference between these: > > - Map<String, Locale> sortedLocales = new TreeMap<String, Locale>(); It happens to be a TreeMap, but all the type declares is that it's a map. > - SortedMap<String, Locale> sortedLocales = new TreeMap<String, Locale>(); It happens to be a TreeMap, but all the type declares is that it's a sorted map. > - Map<String, Locale> sortedLocales = new SortedMap<String, Locale>(); Illegal. SortedMap is an interface. > Or, by the same token: > > - Calendar cal = Calendar.getInstance(); > - Calendar cal = new GregorianCalendar(); > - GregorianCalendar gcal = new GregorianCalendar(); Those last two examples are an exact analogue. The first one is a little different - but a natural extension of the idea. The third version says "I care that this is object is specifically a GregorianCalendar. I am going to do things with it which specifically would not work with any other kind of calendar.". There are times when you might want to say that, but you would strive to be more general - in which case you would use the second version, which says "I care that this object is a Calendar, but i don't care which kind of calendar - it could be Gregorian, Islamic, Japanese Imperial, Darian Jovian, etc". However, in the second form, although you will proceed to write your code calendar-agnostically, you are actually hardcoding the choice of GregorianCalendar there. You could change it later, but you'd have to go through your code and change the constructions. In the first form, you push the calendar-agnosticism even into the creation itself - rather than explicitly using a GregorianCalendar, you let Calendar decide what kind of calendar should be used. Right now, that will presumably always be a GregorianCalendar, but if at some point someone boots up your software on Ganymede (the moon, not the Eclipse release), it should probably be Darian, and by leaving the decision to Calendar.getInstance, you let that happen without having to alter your code. You delegate the decision to someone in a better position to make it. Now, in this particular case, there is a major caveat: different types of Calendar are not interchangeable in the way that different types of Map are, because they represent different ideas. If you're using this Calendar object to handle dates which are definitively in Gregorian (January, February and all that), then you need an actual GregorianCalendar, and you should declare your variables accordingly. To paraphrase Einstein, "everything should be made as generic as possible, but no more so". > If I had to write an exam, clearly articulating the distinction between > those and what the implications of each is, I'd surely make a hash of > it.... That would be a real mistake. You should be making a tree. Have you learned NOTHING? > I can tell if I'm getting a compile error and I can tell if my code is > doing what I want it to do but I still don't always understand WHY one > thing is better than an other or what the real difference is between > things that look very very similar..... The choice between Map and SortedMap here is not one of correctness. It's one of design and style. Good design and style doesn't change the way code runs, but it makes it easier to understand and modify. A huge part of the business of software is understanding and modifying existing code, and so good design and style are valuable. They're investment for the future, rather than the here and now. tom -- build the roof with holes in
From: Lew on 22 May 2010 18:03
Lew wrote: > It's a best practice to add a lot of "potentially useful" cruft to a > type. Implement what you need now, and refactor later if the need arises. Darn it! It's a best practice NOT to add a lot of cruft. NOT to. Somehow I proofread best after I've embarrassed myself. -- Lew |