passing a Factory to a method to create a generic instance

Discussion in 'Java' started by thufir, May 9, 2008.

  1. thufir

    thufir Guest

    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
     
    thufir, May 9, 2008
    #1
    1. Advertising

  2. thufir

    Tom Anderson Guest

    On Fri, 9 May 2008, thufir wrote:

    > 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.

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


    --
    I could tell you a great many more particulars but suppose that you are
    tired of it by this time. -- John Backhouse, Trainspotter Zero
     
    Tom Anderson, May 9, 2008
    #2
    1. Advertising

  3. thufir

    thufir Guest

    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.

    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
     
    thufir, May 9, 2008
    #3
  4. thufir

    Mark Space Guest

    thufir wrote:

    > 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.
     
    Mark Space, May 9, 2008
    #4
  5. thufir

    Tom Anderson Guest

    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
     
    Tom Anderson, May 10, 2008
    #5
  6. thufir

    Tom Anderson Guest

    On Fri, 9 May 2008, Mark Space wrote:

    > thufir wrote:
    >
    >> 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;
    >> }


    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

    --
    Remember Sammy Jankis.
     
    Tom Anderson, May 10, 2008
    #6
  7. thufir

    thufir Guest

    On Sat, 10 May 2008 01:19:13 +0100, Tom Anderson wrote:

    > 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, May 10, 2008
    #7
  8. thufir

    thufir Guest

    On Sat, 10 May 2008 04:48:07 +0000, thufir wrote:

    > 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?

    > rooms = FileUtil.load(roomsFile, new RoomFactory());
    > guests = FileUtil.load(guestsFile, new GuestFactory());
    > FileUtil.output(rooms,new File("out.txt"));
    > }
    > }
    > thufir@arrakis:~/bcit-comp2611-project1$
     
    thufir, May 10, 2008
    #8
  9. thufir

    Mark Space Guest

    Tom Anderson wrote:

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


    > 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().

    >> 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!


    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.
     
    Mark Space, May 10, 2008
    #9
  10. thufir

    Tom Anderson Guest

    On Sat, 10 May 2008, Mark Space wrote:

    > Tom Anderson wrote:
    >
    >>>> 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;
    >>>> }

    >>
    >> 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.


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

    >> 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().


    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.

    >>> 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!

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

    --
    Finals make a man mean; let's fusc up and write!
     
    Tom Anderson, May 10, 2008
    #10
  11. thufir

    Tom Anderson Guest

    On Sat, 10 May 2008, thufir wrote:

    > On Sat, 10 May 2008 01:19:13 +0100, Tom Anderson wrote:
    >
    >> 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.


    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

    --
    Finals make a man mean; let's fusc up and write!
     
    Tom Anderson, May 10, 2008
    #11
  12. thufir

    Mark Space Guest

    Tom Anderson wrote:

    > 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.
     
    Mark Space, May 10, 2008
    #12
  13. thufir

    Tom Anderson Guest

    On Sat, 10 May 2008, Lew wrote:

    > Tom Anderson wrote:
    >> On Sat, 10 May 2008, thufir wrote:
    >>
    >>> On Sat, 10 May 2008 01:19:13 +0100, Tom Anderson wrote:
    >>>
    >>>> 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.

    >>
    >> 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.

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

    --
    I DO IT WRONG!!!
     
    Tom Anderson, May 11, 2008
    #13
  14. thufir

    Tom Anderson Guest

    On Sun, 11 May 2008, Lew wrote:

    > Tom Anderson wrote:
    >
    >> 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.

    >
    > 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.

    > Lew wrote:
    >>> 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.

    >
    > Tom:
    >> Widget or WidgetFactory? In this example (or rather, the hypothetical

    >
    > Both, really, but only because the example is so simple.
    >
    >> 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.

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

    --
    I know you wanna try and get away, but it's the hardest thing you'll
    ever know
     
    Tom Anderson, May 11, 2008
    #14
  15. thufir

    Tom Anderson Guest

    On Sat, 10 May 2008, Mark Space wrote:

    > Tom Anderson wrote:
    >
    >> 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.


    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

    --
    I know you wanna try and get away, but it's the hardest thing you'll
    ever know
     
    Tom Anderson, May 11, 2008
    #15
  16. thufir

    Mark Space Guest

    Tom Anderson wrote:

    > 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.



    >> 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.


    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.
     
    Mark Space, May 12, 2008
    #16
  17. thufir

    Arne Vajhøj Guest

    Mark Space wrote:
    > Tom Anderson wrote:
    >> 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?


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

    Arne
     
    Arne Vajhøj, May 12, 2008
    #17
  18. thufir

    Tom Anderson Guest

    On Sun, 11 May 2008, Mark Space wrote:

    > Tom Anderson wrote:
    >
    >> 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?


    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.

    >> 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.


    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.

    >>> 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.

    >
    > 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.

    >> 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.


    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

    --
    Argumentative and pedantic, oh, yes. Although it's properly called
    "correct" -- Huge
     
    Tom Anderson, May 12, 2008
    #18
  19. thufir

    Tom Anderson Guest

    On Sun, 11 May 2008, Lew wrote:

    > Tom Anderson wrote:
    >
    >> 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.

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

    --
    Any problem in computer science can be solved with another layer of
    indirection. -- David Wheeler
     
    Tom Anderson, May 12, 2008
    #19
  20. thufir

    Mark Space Guest

    Tom Anderson wrote:

    >
    > 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.


    >> Now we have a POJO

    >
    > 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.
     
    Mark Space, May 12, 2008
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    3
    Views:
    1,776
    Roedy Green
    Sep 17, 2005
  2. Medi Montaseri
    Replies:
    17
    Views:
    921
    Medi Montaseri
    Sep 3, 2003
  3. C#
    Replies:
    4
    Views:
    440
  4. Raj Singh
    Replies:
    2
    Views:
    218
    Rick DeNatale
    May 29, 2008
  5. Greg Hauptmann
    Replies:
    9
    Views:
    271
    Loren Segal
    Jun 16, 2008
Loading...

Share This Page