Re: passing a Factory to a method to create a generic instance
On Fri, 9 May 2008, thufir wrote:
On Fri, 09 May 2008 14:30:22 +0100, Tom Anderson wrote:
I've had a look through your code, and it all seems a bit complicated.
I notice that you're planning to use reflection to move data from JDBC
result sets to your objects. That's kind of gross. But if you are going
to do that, you don't need a factory: just pass in the Class object for
the objects you want to create (Guest.class or Room.class), then use
the newInstance method on the class object to create objects to feed to
your reflective loader. You can do this for loading from a file too. No
factories, just reflection. Although really, the Class object is acting
as a factory.
I'd prefer not to use reflection, is that possible? I don't want to make
it complicated.
Good man. You had some reflection in the code you posted, so i was
following up on that. But doing it without reflection is the Right Thing
to do.
Rather than a RoomFactory class which implements Factory, could Room
implement Factory?
Do rooms make other rooms? Not in any hotel i've stayed in.
for:
public List load(Factory fac) {
List l = new ArrayList() ;
for (String[] row : somehowReadDataFromSomewhere()) {
Object obj = fac.make(row) ; //this seems weird
Well, that's what the Factory pattern is, so if you want to use that, get
used to weirdness!
What do you find weird about it?
l.add(obj) ;
}
return l ;
}
Can I do something better than creating Object obj ? Somehow use
generics, perhaps a wildcard ? operator?
Kids today! When i was your age, we made do with good old-fashioned
polymorphism. Now it's generic this, generic that, iPod happy slapping
Justin Timberlake. I don't know. What would Alan Kay say if he could see
all this?
Ahem.
Okay, so, firstly, why do you want something more specific than Object?
How does that help you?
I guess it means you could return a List<Room> or something. Okay, no
problem. We can do this.
Before i go on, i should say that in the classical application of the
Factory pattern, yes, you would make something more specific than an
Object, because the different factories would be making different versions
of something, or the same thing in different ways. Like you might define a
WidgetFactory, then have concrete factories that make Nut, Bolt, Screw,
etc objects, all of which are subtypes of Widget. Or you might have a
GameMapFactory, and have concretes which produce a GameMap by random
generation, picking a preset map, using a fractal, etc. The type of thing
you're making is whatever the most appropriate type to cover all the
possible products is. In your case, there's no common base class of Room
and Guest below Object, so that's what i was using.
Anyway, generics. I'm not really up on this generics stuff, but how about
this:
public interface Factory<T> {
public T make(String[] data) ;
}
public class RoomFactory implements Factory<Room> {
public Room make(String[] data) {
return new Room(data) ;
}
}
public class GuestFactory implements Factory<Guest> {
public Guest make(String[] data) {
return new Guest(data) ;
}
}
public class Loader {
public <T> List<T> load(File f, Factory<T> fac) {
List<T> l = new ArrayList<T>() ;
for (String[] data : readLines(f)) {
T obj = fac.make(data) ;
l.add(obj) ;
}
return l ;
}
}
Does that work?
Thank you for the information! I'm not sure that I'm going to do this,
no points off for what I have now, but it just burns me up to have nearly
identical methods:
loadGuest
loadRoom
loadFoo
which are nearly indistinguishable.
Good. This is a good thing to be annoyed by!
More recent code, cleaned up a bit from what you probably saw:
Yes, this is a *lot* more readable!
thufir@arrakis:~/bcit-comp2611-project1$
thufir@arrakis:~/bcit-comp2611-project1$ cat src/a00720398/util/
FileUtil.java
/**
* FileUtil.java
*/
package a00720398.util;
import java.util.*;
import java.io.*;
import a00720398.data.*;
import a00720398.interfaces.*;
public abstract class FileUtil {
public static void outputRooms(List<Room> rooms, File file){
PrintWriter outputStream = null;
try {
outputStream = new PrintWriter(file);
for (Room room : rooms) {
outputStream.println(room);
}
} catch (Exception e) {
e.printStackTrace();
} finally {
outputStream.close();
}
}
public static void outputGuests(List<Guest> guests, File file){
PrintWriter outputStream = null;
try {
outputStream = new PrintWriter(file);
for (Guest guest : guests) {
outputStream.println(guest);
}
} catch (Exception e) {
e.printStackTrace();
} finally {
outputStream.close();
}
}
The above two methods can also be merged. You don't even need to use
generics, you can just use good, honest polymorphism.
And don't call a PrintWriter outputStream!
public static void outputRooms(List<Room> data){}
public static List<Room> loadRooms(File file) {
List<Room> rooms = new ArrayList<Room>();
List<String> lines = getLines(file);
for (String line : lines) {
List<String> tokens = getTokens(line);
Room room = new Room(tokens);
rooms.add(room);
}
return rooms;
}
public static List<Guest> loadGuests(File file) {
List<Guest> guests = new ArrayList<Guest>();
List<String> lines = getLines(file);
for (String line : lines) {
List<String> tokens = getTokens(line);
Guest guest = new Guest(tokens);
// System.out.println(guest);
guests.add(guest);
}
// System.out.println(guests);
return guests;
}
private static List<String> getTokens (String string){
List<String> tokens = new ArrayList<String>();
Scanner tokenScanner = new Scanner(string);
tokenScanner.useDelimiter("\t");
while (tokenScanner.hasNext()){
String token = tokenScanner.next();
tokens.add(token);
}
return tokens;
}
Something wrong with string.split("\t")?
private static Guest newGuest(String string) {
List<String> guestData = getTokens(string);
Guest guest = new Guest(guestData);
return guest;
}
private static List<String> getLines(File file) {
List<String> lines = new ArrayList<String>();
try {
Scanner scanner = new Scanner(file);
while (scanner.hasNextLine()) {
String line = scanner.nextLine();
Scanner lineScanner = new Scanner(line);
lineScanner.useDelimiter("\n");
while (lineScanner.hasNextLine()) {
String oneLine = lineScanner.next();
lines.add(oneLine);
// System.out.println("*****\n\n\n" + oneLine);
}
}
} catch (FileNotFoundException e) {
e.printStackTrace();
}
return lines;
}
}
Well, i have no idea what that's about. What's this Scanner? What's wrong
with:
private static List<String> getLines(File file) throws IOException {
List<String> lines = new ArrayList<String>() ;
BufferedReader input = new BufferedReader(new FileReader(file)) ;
while (true)
{
String line = input.readLine() ;
if (line == null) break ;
lines.add(line) ;
}
return lines ;
}
?
It's a shame BufferedReader isn't Iterable, as then you could do:
for (String line : new BufferedReader(whatever)) { ... }
And avoid having to read the lines in one go. You could also avoid reading
them all at once (which is generally a good thing, BTW - saves on memory)
by making your loop in load go over the lines coming out of a
BufferedReader rather than a list of String. You can't use the
sugar-coated for loop for that, though.
Also, i'm seeing a lot of static in your code. Not a lot of object
thinking. Have you come from a procedural language?
tom
--
Heinlein has done more to harm SF than has any other writer, I think. --
PKD