passing a Factory to a method to create a generic instance

[thufir]

The two nearly identical methods, loadGuests and loadRooms, I would like
to consolidate by passing in a Factory. However, I haven't found much
documentation on what that looks like.

Guest and Room implement Factory, but that's just part of the picture.

public Guest new() {return new Guest());} //in Guest.java
public Room new() {return new Room());} //in Room.java

Is the idea to make a single method, outside of Room and Guest, which
returns an instance of, well, type <T>?

But, how? I understand that one approach is to pass a Factory to
FileUtils.load(), which is fine, but how is that Factory instance first
created? Where's it created, in the driver which invokes FileUtils.load
()?

Here's what I have, it's just the top method which I'm working on right
now:

thufir@arrakis:~/bcit-comp2611-project1$
thufir@arrakis:~/bcit-comp2611-project1$
thufir@arrakis:~/bcit-comp2611-project1$ cat src/a00720398/interfaces/
Factory.java

package a00720398.interfaces;


public interface Factory <T>{
T make();
}
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 <E> List<E> load(File file) {
List<E> foos = new ArrayList<E>();
List<String> lines = getLines(file);
for(String line : lines){
// <E> foo = new <E>(); //how do I pass in a
// //factory to return an "E"?
}
return new ArrayList<E>();
}

/*
a00720398/util/FileUtil.java:31: cannot find symbol
symbol : class T
location: class a00720398.util.FileUtil
public static void factoryTest(Factory<T> factory) {}
^
1 error */
public static void factoryTest(Factory<T> factory) {} //

/**

interface Factory<T> { T make();}
public <T> Collection<T> select(Factory<T> factory, String statement) {
Collection<T> result = new ArrayList<T>();
/* run sql query using jdbc
for (/* iterate over jdbc results ) {
T item = factory.make();
/* use reflection and set all of item’s fields from sql results
result.add(item);
}
return result;
}
You can call this either as
select(new Factory<EmpInfo>(){ public EmpInfo make() {
return new EmpInfo();
}}
, â€selection stringâ€);


public static <T> factory<T>(T type) {
return new List<T>(type);
}



@see http://www.ibm.com/developerworks/java/library/j-
djc02113.html

Utilities.make(Integer(0)) //called from driver
<T extends Object> public static <T> factory (T first) {
return ew List<T>(first);
}


class Utilities {
<T extends Object> public static List<T> make(T first) {
return new List<T>(first);
}
}

*/

//this method is a repaid of loadGuests
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();
System.out.println(rooms);
}

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);
}
return guests;
}

private static List<String> getTokens (String string){
List<String> tokens = new ArrayList<String>();
Scanner lineScanner = new Scanner(string);
while (lineScanner.hasNext()){
String token = lineScanner.next();
tokens.add(token);
}
return tokens;
}

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);
}
}
} catch (FileNotFoundException e) {
e.printStackTrace();
}
return lines;
}
}
thufir@arrakis:~/bcit-comp2611-project1$




thanks,

Thufir

 

[Tom Anderson]

The two nearly identical methods, loadGuests and loadRooms, I would like
to consolidate by passing in a Factory. However, I haven't found much
documentation on what that looks like.

Do you have a copy of 'Design Patterns', by Gamma, Helm, Johnson and
Vlissides? If not, get one. Your university library will probably have it.
If you want to use patterns, it's really worth having!

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.

However, if you want to use a factory properly, it looks like:

public class Room {
public Room(String[] data) {
...
}
}

public class Guest {
public Guest(String[] data) {
...
}
}

public interface Factory {
public Object make(String[] data) ;
}

public class RoomFactory implements Factory {
public Object make(String[] data) {
return new Room(data) ;
}
}

public class GuestFactory implements Factory {
public Object make(String[] data) {
return new Guest(data) ;
}
}

public List load(Factory fac) {
List l = new ArrayList() ;
for (String[] row : somehowReadDataFromSomewhere()) {
Object obj = fac.make(row) ;
l.add(obj) ;
}
return l ;
}

Obviously you'll need to add all your generics and actual implemention and
whatnot.

A smell in this code is the use of arrays of strings as constructor
arguments. This is not typesafe or self-documenting. I think i'd refactor
so that the Room and Guest objects took proper parameters (so Room takes
int floor, int number, int numBeds, boolean enSuite, etc), then move the
parsing code to the factories.

tom

PS Does the code you posted even compile? Looks a total mess to me.

 

[thufir]

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.

Rather than a RoomFactory class which implements Factory, could Room
implement Factory?

for:

public List load(Factory fac) {
List l = new ArrayList() ;
for (String[] row : somehowReadDataFromSomewhere()) {
Object obj = fac.make(row) ; //this seems weird
l.add(obj) ;
}
return l ;
}


Can I do something better than creating Object obj ? Somehow use
generics, perhaps a wildcard ? operator?

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.

More recent code, cleaned up a bit from what you probably saw:


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();
}
}

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;
}

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;
}
}
thufir@arrakis:~/bcit-comp2611-project1$
thufir@arrakis:~/bcit-comp2611-project1$




thanks,

Thufir //getting some zzz's

 

[Mark Space]

I'd prefer not to use reflection, is that possible? I don't want to make
it complicated.

Rather than a RoomFactory class which implements Factory, could Room
implement Factory?

I didn't have time to read through all the code thoroughly, but a couple
of ideas occur to me.

First, you might want to define an interface for loading data, rather
than define a loadGuest, loadRoom, loadEtc.

public interface AssignmentLoadable {
void assignmentLoad( IOStream io ) throws IOException;
}

Now you can take all your objects, add this interface, and load them all
the same way.

> public static List<AsignmentLoadable> loadAll(File file) {
> List<AsignmentLoadable> al =
> new ArrayList<AsignmentLoadable>();
> //List<String> lines = getLines(file);
> IOStream ios = new FileIOStream( file );
>
> while( (AssignmentLoadable a = getNextObject( ios )) != null )
> a.assignmentLoad( ios );
> al.add( a );
> }
> return al;
> }

This is called a mix-in interface, btw, because it's an interface that's
easy to add to a class.

For the factory pattern, it might be better to use package private
constructors, then make the factory object part of the package, so it
has access to the constructors. Static methods are ok, but they aren't
inheritable and therefore can't be added to interfaces, so they're a
little awkward to work with.

public class Room implements AssignmentLoadable {
Room() {} // NOTE: package private
// etc.
}

public class HotelFactory {
public List<AssignmentLoadable > newGuestList( File file ) { // etc.
}
}

You'll need to do a little work massaging AssignmentLoadable into
whatever top level class you want. I took the cheap and easy way out. I
didn't make HotelFactory itself generic because I want a concrete object
at the top level so it's easy to work with.

Now you have to parse the file into classes. This means matching up
strings with objects. A really sophisticated parser might read it's
configuration out of a separate file, so you could confiure it on the fly.

<parse-token>
<parse-string>Room:</parse-string>
<parser-class>my.parser.RoomParser</parser-class>
<object>a00720398.etc.Room</object>
</parse-token>

But for your assignment you might just want to tell the HotelFactory
that the string "Room:" is associated with the class "a00720398.Room"
for example. This gives you a bit of flexibility. It also might be
complete overkill for your assignment. Just building the Strings and
classes into HotelFactory as literals would probably work ok. However,
using this setter (and a few others) to configure the HotelFactory
object does give you the ability to inject a test object into the
HotelFactory, which might be handy if you run into real trouble
debugging it.

public class HotelFactory {

Map<String,Class<? extends AsignmentLoadable>> parserStrings =
new HashMap<String,Class<? extends AssignmentLoadable>>();

public void setRoomParsing( String s,
Class<? extends AssignmentLoadable> c )
{
parserStrings.put( s, c );
}

}

Set this up externally when you make the Factory:

HotelFactory h = new HotelFactory();
h.setRoomParseing( "Room:", a00720398.Room.class );
// etc. for other setters...
List guests = h.newGuestList( myFile );


Then read your input file, match up strings with classes, and load the
classes up with the assginmentLoad() method, something like this:

class HotelFactory {
public List<AssignmentLoadable> newGuestList( File file )
throws FileNotFoundException,
IOException,
InstantiationException,
IllegalAccessException
{

List<AssignmentLoadable> guestList = new
LinkedList<AssignmentLoadable>();

FileInputStream fios = new FileInputStream(file);
BufferedReader bin = new BufferedReader( new
InputStreamReader(fios) );

String line;
while( (line = bin.readLine()) != null )
{
Class<AssignmentLoadable> c
= (Class<AssignmentLoadable>) parserStrings.get( line );
AssignmentLoadable a = c.newInstance();
a.assignmentLoad(fios);
guestList.add(a);
}
return guestList;
}

// etc.

}


There are probably a couple of good design patterns that I'm missing
(parsing is not my strong suit) but this will hopefully at least give
you some ideas.

 

[Tom Anderson]

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

 

[Tom Anderson]

I didn't have time to read through all the code thoroughly, but a couple of
ideas occur to me.

First, you might want to define an interface for loading data, rather than
define a loadGuest, loadRoom, loadEtc.

public interface AssignmentLoadable {
void assignmentLoad( IOStream io ) throws IOException;
}

Now you can take all your objects, add this interface, and load them all the
same way.

On a technical note, i think the type needs to be <? implements
AssignmentLoadable> rather than just <AssignmentLoadable>, otherwise the
List you get back isn't going to be much use. Or even for the method to be
generic, with a parameter type <T implements AssignmentLoadable>.

On a style note, my beef with this design is that you have the potential
to have instantiated but uninitialised objects floating about, which makes
me nervous. I'd rather do it all properly in the constructor myself, so
that we have a guarantee that in all cases, instances are properly
initialised.

Also, i don't get what getNextObject(ios) is doing, or how it knows what
class to instantiate.

This is called a mix-in interface, btw, because it's an interface that's easy
to add to a class.

For the factory pattern, it might be better to use package private
constructors, then make the factory object part of the package, so it
has access to the constructors.

I like this idea!

Static methods are ok, but they aren't inheritable and therefore can't
be added to interfaces, so they're a little awkward to work with.

Also, totally unhelpful wrt the Factory pattern.

public class Room implements AssignmentLoadable {
Room() {} // NOTE: package private
// etc.
}

public class HotelFactory {
public List<AssignmentLoadable > newGuestList( File file ) { // etc.
}
}

You'll need to do a little work massaging AssignmentLoadable into whatever
top level class you want. I took the cheap and easy way out. I didn't make
HotelFactory itself generic because I want a concrete object at the top level
so it's easy to work with.

Now you have to parse the file into classes. This means matching up strings
with objects. A really sophisticated parser might read it's configuration
out of a separate file, so you could confiure it on the fly.

<parse-token>
<parse-string>Room:</parse-string>
<parser-class>my.parser.RoomParser</parser-class>
<object>a00720398.etc.Room</object>
</parse-token>

But for your assignment you might just want to tell the HotelFactory that the
string "Room:" is associated with the class "a00720398.Room" for example.
This gives you a bit of flexibility. It also might be complete overkill for
your assignment. Just building the Strings and classes into HotelFactory as
literals would probably work ok. However, using this setter (and a few
others) to configure the HotelFactory object does give you the ability to
inject a test object into the HotelFactory, which might be handy if you run
into real trouble debugging it.

public class HotelFactory {

Map<String,Class<? extends AsignmentLoadable>> parserStrings =
new HashMap<String,Class<? extends AssignmentLoadable>>();

public void setRoomParsing( String s,
Class<? extends AssignmentLoadable> c )
{
parserStrings.put( s, c );
}

}

Set this up externally when you make the Factory:

HotelFactory h = new HotelFactory();
h.setRoomParseing( "Room:", a00720398.Room.class );
// etc. for other setters...
List guests = h.newGuestList( myFile );

Then read your input file, match up strings with classes, and load the
classes up with the assginmentLoad() method, something like this:

class HotelFactory {
public List<AssignmentLoadable> newGuestList( File file )
throws FileNotFoundException,
IOException,
InstantiationException,
IllegalAccessException
{

List<AssignmentLoadable> guestList = new
LinkedList<AssignmentLoadable>();

FileInputStream fios = new FileInputStream(file);
BufferedReader bin = new BufferedReader( new InputStreamReader(fios) );

String line;
while( (line = bin.readLine()) != null )
{
Class<AssignmentLoadable> c
= (Class<AssignmentLoadable>) parserStrings.get( line );
AssignmentLoadable a = c.newInstance();
a.assignmentLoad(fios);
guestList.add(a);
}
return guestList;
}

// etc.

}

There are probably a couple of good design patterns that I'm missing
(parsing is not my strong suit) but this will hopefully at least give
you some ideas.

I didn't get the impression the OP needed to do all that for his
assignment, but if he does, that's solid stuff.

Pattern-wise, er, i'm all out for tonight!

tom

 

[thufir]

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.

Does there have to be a Nut factory, or can I just use the Widget factory?

I'm not seeing the advantage of WidgetFactory, because I don't seem able
to use it. For me, DataFactory rather than WidgetFactory:



Guest extends Data //Data is just the name of package
Room extends Data //non-inspiring name

public class DataFactory implements Factory<Data> {
public Data make(List<String> data) {return new Data(data);}
}

public interface Factory <T>{
public T make(List<String> data);
}


This is the error:

a00720398/bedz/Bedz.java:24: incompatible types
found : java.util.List<a00720398.data.Data>
required: java.util.List<a00720398.data.Room>
rooms = FileUtil.load(roomsFile, new DataFactory());
^
1 error



thufir@arrakis:~/bcit-comp2611-project1$
thufir@arrakis:~/bcit-comp2611-project1$ cat src/a00720398/bedz/Bedz.java
package a00720398.bedz;


public class Bedz {
//deleted some stuff

public static void main (String[] args){

/* how do I pass a DataFactory()?
rooms = FileUtil.load(roomsFile, new DataFile()); //error
*/
rooms = FileUtil.load(roomsFile, new RoomFactory());
guests = FileUtil.load(guestsFile, new GuestFactory());
FileUtil.output(rooms,new File("out.txt"));
}
}
thufir@arrakis:~/bcit-comp2611-project1$


thanks,

Thufir

 

[thufir]

This is the error:

a00720398/bedz/Bedz.java:24: incompatible types found :
java.util.List<a00720398.data.Data> required:
java.util.List<a00720398.data.Room>
rooms = FileUtil.load(roomsFile, new DataFactory());
^
1 error



thufir@arrakis:~/bcit-comp2611-project1$
thufir@arrakis:~/bcit-comp2611-project1$ cat
src/a00720398/bedz/Bedz.java package a00720398.bedz;


public class Bedz {
//deleted some stuff

public static void main (String[] args){

/* how do I pass a DataFactory()?
rooms = FileUtil.load(roomsFile, new DataFile()); //error
*/

Whoops. What's problematic:

rooms = FileUtil.load(roomsFile, new DataFactory()); //error

What use is the DataFactory? Or am I mis-using it?

 

[Mark Space]

On a style note, my beef with this design is that you have the potential
to have instantiated but uninitialised objects floating about, which
makes me nervous. I'd rather do it all properly in the constructor
myself, so that we have a guarantee that in all cases, instances are
properly initialised.

a.assignmentLoad() is supposed to initialize the object, completely,
from the data read from ios. The only way that an object would be
improperly initialized would be if the data was bad or there was an IO
error in the middle of the stream. Or that's what I'm thinking.

Also, i don't get what getNextObject(ios) is doing, or how it knows what
class to instantiate.

It's basically the same as my code below this (which I snipped). Read a
token from the stream, associate that token with a class, instantiate
the class with newInstance().

>
> I like this idea!

Thanks. I got this idea primarily from working with JUnit. JUnit makes
unit test objects that are part of the same package as the classes they
test. So you have access to package private methods, although those
methods are not available to other classes outside the package. It's a
handy little back-door into a class.

I think of packages now as Java's way of implementing behavior like C++
"friend" classes.

 

[Tom Anderson]

a.assignmentLoad() is supposed to initialize the object, completely,
from the data read from ios. The only way that an object would be
improperly initialized would be if the data was bad or there was an IO
error in the middle of the stream. Or that's what I'm thinking.

Or if someone else called the constructor and didn't get round to calling
assignmentLoad. That's the kind of thing that worries me.

It's basically the same as my code below this (which I snipped). Read a
token from the stream, associate that token with a class, instantiate
the class with newInstance().

Okay, got it.

How about doing the assignmentLoad in the constructor? That would mean you
have to mass the ios into the constructor, which means using some more
complicated reflection to invoke it, rather than newInstance. I think like
this:

Class<AssignmentLoadable> cl = determineObjectClass() ;
AssignmentLoadable a = cl.getConstructor(IOStream.class).newInstance(ios) ;

Plus various unexciting catch blocks, of course. That isn't so bad, is it?
It means one less method on the domain objects, and eliminates the
possibility of constructed but unloaded objects existing.

Thanks. I got this idea primarily from working with JUnit. JUnit makes unit
test objects that are part of the same package as the classes they test. So
you have access to package private methods, although those methods are not
available to other classes outside the package. It's a handy little
back-door into a class.

I think of packages now as Java's way of implementing behavior like C++
"friend" classes.

Yes, exactly. Hey, turns out C++ still has some good ideas after all!

tom

 

[Tom Anderson]

Does there have to be a Nut factory, or can I just use the Widget factory?

I'm not seeing the advantage of WidgetFactory, because I don't seem able
to use it.

WidgetFactory is abstract. NutFactory etc are concrete implementations of
it. Sorry if i haven't explained this clearly.

abstract class Widget {
}

class Nut extends Widget {
}

class Bolt extends Widget {
}

abstract class WidgetFactory {
abstract Widget make() ;
}

class NutFactory extends WidgetFactory {
Widget make() {
return new Nut() ;
}
}

class BoltFactory extends WidgetFactory {
Widget make() {
return new Bolt() ;
}
}

// a usage example
class CratePacker
{
Crate pack(int number, WidgetFactory fac) {
Crate c = new Crate() ;
for (int i = 0 ; i < number ; ++i)
c.add(fac.make()) ;
return c ;
}
}

The point is to be able to pack crates of widgets with one method, which
can be parameterised with a factory which defines the kind of widget.

For me, DataFactory rather than WidgetFactory:



Guest extends Data //Data is just the name of package
Room extends Data //non-inspiring name

public class DataFactory implements Factory<Data> {
public Data make(List<String> data) {return new Data(data);}
}

public interface Factory <T>{
public T make(List<String> data);
}

Nope. You can't make a Data directly - that's why you need separate
subclasses of factory for Guest and Room. This is, in fact, the whole
reason for the factory pattern.

This is the error:

a00720398/bedz/Bedz.java:24: incompatible types
found : java.util.List<a00720398.data.Data>
required: java.util.List<a00720398.data.Room>
rooms = FileUtil.load(roomsFile, new DataFactory());
^
1 error



thufir@arrakis:~/bcit-comp2611-project1$
thufir@arrakis:~/bcit-comp2611-project1$ cat src/a00720398/bedz/Bedz.java
package a00720398.bedz;


public class Bedz {
//deleted some stuff

public static void main (String[] args){

/* how do I pass a DataFactory()?
rooms = FileUtil.load(roomsFile, new DataFile()); //error
*/
rooms = FileUtil.load(roomsFile, new RoomFactory());
guests = FileUtil.load(guestsFile, new GuestFactory());
FileUtil.output(rooms,new File("out.txt"));
}
}
thufir@arrakis:~/bcit-comp2611-project1$

I think you've just proved the point. You need a RoomFactory and a
GuestFactory, which are both subtypes of some abstract DataFactory.
Apologies for not explaining this clearly - but i think you really should
spend some time reading existing documentation of the factory pattern.

tom

 

[Mark Space]

How about doing the assignmentLoad in the constructor? That would mean
you have to mass the ios into the constructor, which means using some
more complicated reflection to invoke it, rather than newInstance. I
think like this:

Class<AssignmentLoadable> cl = determineObjectClass() ;
AssignmentLoadable a = cl.getConstructor(IOStream.class).newInstance(ios) ;

So why not use the constructor:

For me, there are two issues. First, a class may have more than one
constructor, and it can be difficult to know, based on just data input,
exactly which constructor should be called.

Second, constructors are not inherited and can't be members of an
interface. Thus, like statics, they're also inconvenient to use. You
can't be assured that a sub-class of another class implements all the
same signatures that the super class does (because they aren't inherited).

AssignementLoadable otoh is guaranteed to exist, because it's a public
member of the super class. The sub-class must have the assignmentLoad
method, either by overloading it or by inheritance.

Using just the default constructor makes it easy to know which
constructor to call: it's always the default one. If a class doesn't
have a default constructor, then I can't instantiate it with this
routine, of course, but that's as close as I can't get to a guaranteed
constructor.

Then a series of getters and setters will configure the class. While
there's other ways to do this (Serializable, for example), assuming
setters seems the easiest way of reading a class from a arbitrary
format. You just assume each data item has a corresponding setter, a
simple 1-to-1 mapping.

I don't feel there's any danger here that's different that using a Java
bean, which is basically initialized and created the same way. The
object is correctly created by it's constructor. There's no "danger"
past that point.

Once you get into Serializable a bit more, I think it's possible to
create immutable objects and such, in a single method call, not split
like mine. However this is a lot more sophisticated use of the
language. Mine is just a simple example of one way to do it.

 

[Tom Anderson]

Closures? We don't need no steenkin' closures!

Heh! An option i didn't mention was to use an anonymous class for the
factory:

Crate boxOBolts = CratePacker.pack(200, new WidgetFactory() {
Widget make() {return new Bolt() ;}
}) ;

Which is basically a closure in a fancy costume, and would mean you didn't
need the concrete factory classes.

With first-class functions, you could even make the CratePacker take a
FUNCTION FROM void TO Widget or whatever it'd be called, and then pass in
a Widget class's constructor. That's what i do in python.

In this simple example, Wdiget should be an interface rather than a
class, but that in no wise detracts from the main point of the example.

Widget or WidgetFactory? In this example (or rather, the hypothetical
expanded real-world case it's a cartoon of), whether Widget is an
interface or a class depends on design considerations related to what
Widgets do, rather than the use of the factory pattern - do they have code
they can share? Even if not, if the main thing that a Bolt is is a kind of
Widget, i'd say use an abstract base class, not an interface, since it
communicates a stronger sense of is-a. WidgetFactory, though, yes, that
could well be an interface.

tom

 

[Tom Anderson]

Constructors aren't methods in Java, therefore would not be eligible
even if we did get closures.

They might be - if it was decided that they would be. I doubt it'd be hard
to implement.

To my eye, the pluggable (and perhaps anonymous) class syntax is easier
to read and maintain after the fact.

I would agree that the conventional style is more comprehensible. But i
don't think the anonymous class version is any better than closures - it's
just closures with a horrible syntax.

I fear that Java will grow from a WORA (Write Once, Run Anywhere)
language to a WORN (Write Once, Read Never) language.

I suspect this aspect of a program is more to do with the programmer than
the language - recall the old saying that a Real Programmer can write a
FORTRAN program in any language. Still, some languages do make it easier
than others.

Both, really, but only because the example is so simple.


Leaving aside the extreme and controversial view that one should never use a
base class, a view that I do not hold, BTW, there is no difference in
"communication ... of 'is-a'" between inheriting from a class vs. inheriting
from an interface. Both equally express inheritance.

I disagree. When i come across a class that extends another class, i
immediately know that its primary purpose is to be a refinement of that
base class. When a class implements an interface, it could mean anything.

In fact, where one does use an abstract base class, that abstract base
class itself had better freaking implement an interface that specifies
the exact same abstract methods. So the implementing classes actually
will 'be-a' type abstract base class and also 'be-a' type interface.

Seriously? So the only time it's okay to use an abstract base class is
when it's of the FooImpl variety? A class can never define a type in its
own right?

The reason to choose a base class like WidgetFactory has absolutely
nothing so idiosyncratic and subjective as whether it "communicates a
stronger sense of is-a". It has to do with whether one should root the
implementing classes at a single point, and whether that point should
include a chunk of the implementation of its own super-interface.
These are objective and non-aesthetically-motivated criteria.

LOL.

tom

 

[Tom Anderson]

So why not use the constructor:

For me, there are two issues. First, a class may have more than one
constructor, and it can be difficult to know, based on just data input,
exactly which constructor should be called.

I'm thinking there would be a defined constructor signature that would be
used (as implied in my code). There wouldn't be any need to choose between
options.

Second, constructors are not inherited and can't be members of an
interface. Thus, like statics, they're also inconvenient to use. You
can't be assured that a sub-class of another class implements all the
same signatures that the super class does (because they aren't
inherited).

That's true, but ...

AssignementLoadable otoh is guaranteed to exist, because it's a public
member of the super class. The sub-class must have the assignmentLoad
method, either by overloading it or by inheritance.

Using just the default constructor makes it easy to know which
constructor to call: it's always the default one.

.... if a class doesn't have a no-args constructor ...

If a class doesn't have a default constructor, then I can't instantiate
it with this routine, of course, but that's as close as I can't get to a
guaranteed constructor.

.... exactly. You have to make a leap of faith that the constructor you
want is there, just as i have to. Just because your leap of faith doesn't
take any parameters doesn't make it any more certain!

As an aside, does the introduction of generics, and the way they're used
by Class, mean that something along the lines of a constructor interface
would be useful? Before generics, even if had been was a way to require
that all implementations of an interface declared a specified constructor,
there was no way to make use of it - you call a constructor using an
explicit type literal, where polymorphism doesn't come into play, or via
reflection, where there's no type safety anyway. Can we use generics to
make this possible? Say we could declare constructors in interfaces:

interface AssignmentLoadable {
public AssignmentLoadable(IOStream ios) ;
}

And have them enforced in implementations, can we do something like:

<T implements AssignmentLoadable> T create(Class<T> cl, IOStream ios) {
// somehow invoke the constructor! like one of:

// looks like an anonymous class declaration
return new AssignmentLoadable(ios) cl ;

// pretend new is a static method:
return AssignmentLoadable.new(cl, ios) ;

// python style - single namespace for vars and types
return new cl(ios) ;
}

These syntagma are all pretty horrible. But does the idea work?

At they bytecode level, no; the 'new' bytecode can only reference classes
from the constant pool, not the stack. You'd need a new 'dynamicnew' or
some such bytecode that does so. Without that, this code would have to be
expanded into some reflective calls.

Then a series of getters and setters will configure the class. While
there's other ways to do this (Serializable, for example), assuming
setters seems the easiest way of reading a class from a arbitrary
format. You just assume each data item has a corresponding setter, a
simple 1-to-1 mapping.

Hang on, i thought you were configuring the class through
AssignmentLoadable.load? This sounds like you've gone reflective.

I don't feel there's any danger here that's different that using a Java
bean, which is basically initialized and created the same way.

Yes. Java Beans are also unholy!

The object is correctly created by it's constructor.

Except it isn't, because the constructor doesn't have the information it
needs to create the object. Yes, it allocates memory for it and clears its
fields and runs the instance initialiser, but that's only creation in a
trivial sense. If i do Room.class.newInstance(), i have a Room object, but
i don't have a room - it doesn't have a number, a floor, or a number of
beds.

I know that in correct use, you'll go straight from instantiation to
initialisation, so the period of Room-that-is-not-a-room is vanishingly
small, and no danger can occur. The problem arises when the use is
incorrect, and uninitialised Rooms can escape to unwitting code. In our
discussion of operator overloading, you said "I like to design things that
can't fail" - where failure is completely impossible by design. That's
what i'm trying to get at here.

tom

 

[Mark Space]

I'm thinking there would be a defined constructor signature that would
be used (as implied in my code). There wouldn't be any need to choose
between options.

How many guest are allowed one room? Two? Three? Four? Five?

public class Room
{
public Room( String guest1, String guest2 ) {}
public Room( String guest1, String guest2, String guest2 ) {}
public Room( String guest1, String guest2, String guest2, String
guest2 ) {}
public Room( String guest1, String guest2, String guest2, String
guest2, String guest2 ) {}

}

Now what if you the hotel very suddenly has to put six or seven in one
room for a large convention? And you've got several subclasses of Room?

And what if next week they want to put 8 in one room?

POJO is good, but any design paradigm can be taken too far. Mutable
objects with getters and setters have their place too, and are very
practical most of the time.

One thing you might want consider:

public ImmutableRoomView
{
public ImmutableRoomView( Room r ) {}
}

Now we have a POJO object that uses our bean as builder.

As an aside, does the introduction of generics, and the way they're used
by Class, mean that something along the lines of a constructor interface
would be useful? Before generics, even if had been was a way to require
that all implementations of an interface declared a specified
constructor, there was no way to make use of it - you call a constructor
using an explicit type literal, where polymorphism doesn't come into
play, or via reflection, where there's no type safety anyway. Can we use

I'm not sure. There's no run time type for generics or parameterized
types, so I'm not sure how you'd detect the generic information.

Hang on, i thought you were configuring the class through
AssignmentLoadable.load? This sounds like you've gone reflective.

Good point. assignmentLoad() would have access to the class's private
fields. I'm used to thinking about beans and Swing objects.

I know that in correct use, you'll go straight from instantiation to
initialisation, so the period of Room-that-is-not-a-room is vanishingly
small, and no danger can occur. The problem arises when the use is
incorrect, and uninitialised Rooms can escape to unwitting code. In our
discussion of operator overloading, you said "I like to design things
that can't fail" - where failure is completely impossible by design.
That's what i'm trying to get at here.

See the builder example above, and Lew's point about using builder
objects and throwing errors.

If possible, an object should be constructed in a legal state. If not,
you'll have to check for errors at runtime. As long as this is
encapsulated in the class, it's still safe.

public void addGuest( String name ) {
if( number == 0 || floor == 0 || beds ==0 ) {
throw new RuntimeException( "You blew it pal." );
}
// ..

Now you can't add a guest to an uninitialized room.

 

[Arne Vajhøj]

How many guest are allowed one room? Two? Three? Four? Five?

public class Room
{
public Room( String guest1, String guest2 ) {}
public Room( String guest1, String guest2, String guest2 ) {}
public Room( String guest1, String guest2, String guest2, String
guest2 ) {}
public Room( String guest1, String guest2, String guest2, String
guest2, String guest2 ) {}

}

Now what if you the hotel very suddenly has to put six or seven in one
room for a large convention? And you've got several subclasses of Room?

And what if next week they want to put 8 in one room?

Java has supported variable number of arguments since 1.5, so that
specific problem should be solvable.

Arne

 

[Tom Anderson]

How many guest are allowed one room? Two? Three? Four? Five?

Room.getNumBeds() * Room.getBedSize()!

public class Room
{
public Room( String guest1, String guest2 ) {}
public Room( String guest1, String guest2, String guest2 ) {}
public Room( String guest1, String guest2, String guest2, String guest2 ) {}
public Room( String guest1, String guest2, String guest2, String guest2, String guest2 ) {}
}

Now what if you the hotel very suddenly has to put six or seven in one room
for a large convention? And you've got several subclasses of Room?

And what if next week they want to put 8 in one room?

The one constructor that i want Room to support is one that takes an
IOStream, just like your AssignmentLoadable.load interface. How were you
planning to solve this problem?

I assume that the Room constructor would be responsible for determining
the number of guests and reading their details from the stream.

POJO is good, but any design paradigm can be taken too far. Mutable
objects with getters and setters have their place too, and are very
practical most of the time.

One thing you might want consider:

public ImmutableRoomView
{
public ImmutableRoomView( Room r ) {}
}

Now we have a POJO

You keep using that word. I do not think it means what you think it means.

object that uses our bean as builder.

I'd call ImmutableRoomView Room and Room RoomBuilder (or RoomSpec - if you
call RoomThinger.build() to get a Room, it's a builder, but if you pass a
RoomThinger to Room's constructor, it's a spec), but there you go.

Anyway, i don't think i argued for immutable rooms. But having public
setters for things like room number and floor seems questionable to me.

Ooh, and if you must pass a variable number of guests in to the
constructor, that's easy:

class Party implements Set<Guest>

class Room {
public Room(Party pty, etc)
}

! Only kidding.

I'm not sure. There's no run time type for generics or parameterized
types, so I'm not sure how you'd detect the generic information.

You have the actual Class object, that's how. And you have a generic
guarantee that the Class that's been passed in is something that
implements AssignmentLoadable. That feels like it should be enough, but
i'm not sure.

Good point. assignmentLoad() would have access to the class's private
fields. I'm used to thinking about beans and Swing objects.

Nothing a couple of litres of overproof rum can't cure!

The combination of a bean-like, reflectively-configured RoomBuilder/Spec
object and a 'POJO' Room which you make with it, which i now realise is
what you're getting at, sounds like an excellent way forward to me.

Although it still doesn't solve our original problem of refactoring Room-
and Guest-loading code into one method. Unless the Room/GuestBuilder is
instantiated reflectively too? And the code for configuring them works
equally well with Guests and Rooms? Could be done, i guess. I still think
good old fashioned polymorphism is simpler, though.

See the builder example above, and Lew's point about using builder objects
and throwing errors.

If possible, an object should be constructed in a legal state. If not,
you'll have to check for errors at runtime. As long as this is encapsulated
in the class, it's still safe.

public void addGuest( String name ) {
if( number == 0 || floor == 0 || beds ==0 ) {
throw new RuntimeException( "You blew it pal." );
}
// ..

Now you can't add a guest to an uninitialized room.

No - but you can still write a program which tries to do so. I want to
make that statically impossible.

Which is why i'm completely down with the builder idea.

Although i think we might still need a BuilderFactory.

And i definitely need at least a litre of overproof rum.

tom

 

[Tom Anderson]

Where one absolutely must separate building an object from its
construction, I'm aware of two patterns, the Builder pattern (think
StringBuilder), often combined with the Factory pattern, and the
WhatchaMaCallIt pattern - throw IllegalStateException for any attempt to
operate an unbuilt object.

I had the latter in mind, but it hadn't occurred to me to use Builder
here. As Mark went on to expound, it offers a pretty straightforward way
of dealing with construction in this situation. Good idea.

Although the Factory option here is also straightforward.

The objections to every pattern so far proffered amount to the same
thing. There arise scenarios where initialization is just done well
after construction, and those scenarios carry risk that must be managed
through extra code and effort. Properly architected, that effort comes
at the root of the difficulty and not patched onto the surface.

Constructors really work best when limited to "only creation in a
trivial sense".

I'm not sure i agree with this. In fact, i'm sure i don't agree with this.

It turns out that jamming all the building phase into construction
carries risk, too, and that risk requires management through code and
effort. Unfortunately, many programs do not evince that care, and do
things like call overridable methods in the constructor of a class that
is intended as a base class, or create side effects from the constructor
that are hard to unroll if construction fails.

Good points.

I suspect it is easier to manage the risk of separate construction and
build phases than it is the risk of monolithic construction.

Which i suppose is the whole point of the Builder pattern!

What we have is a tension between an all-at-once interface to
construction, which gives us nice safety and certainty guarantees, but
puts a huge load on the user, and an incremental approach, which is
friendlier, but vulnerable to mistakes. The essence of the builder pattern
is that it lets us present one interface to the object and another one to
the user. It's effectively a large-scale Adapter pattern. Which should
really come as no surprise - see sig!

tom

 

[Mark Space]

The one constructor that i want Room to support is one that takes an
IOStream, just like your AssignmentLoadable.load interface. How were you
planning to solve this problem?

I wasn't. And, someone has written the "all-in-one" constructor for me:

in = new ObjectInputStream( inputStream );
room = (Room)in.readObject();

will do that. So if I had to I'd use some variation on that method. The
OP wanted a way to load objects with a single load method; that's what I
was trying to give him.

<http://java.sun.com/developer/technicalArticles/Programming/serialization/>

I also like the idea of making guests a separate object which are then
added to a Room. Makes sense to me.

You keep using that word. I do not think it means what you think it means.

I'm used to POJO as a contrast to things like EJB. POJO objects also
like to be fully constructed by their CTOR, as opposed to being
configured by setters, thus eliminating the possibility of an illegal state.

You're right though, the Rooms object was a POJO too, as are most
ordinary JavaBeans.


My opinion is that all types of design patterns are needed, and should
be used. Trying to force a pattern onto a design is going to make the
design brittle, hard to maintain, etc. Use builders where they are
needed. Use POJO were it's needed. Use EJB where it's needed.