Bug 35687 - Improvement of record instanciation
Summary: Improvement of record instanciation
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: 3.0-dev
Hardware: PC Windows 2000
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-07-11 15:29 UTC by Yegor Kozlov
Modified: 2011-06-24 10:01 UTC (History)
0 users



Attachments
RecordFactory added to instanciate pp records (9.36 KB, application/x-zip-compressed)
2005-07-11 15:30 UTC, Yegor Kozlov
Details
cvs diff for the previously attached jar (22.91 KB, text/plain)
2005-07-11 15:52 UTC, Yegor Kozlov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yegor Kozlov 2005-07-11 15:29:45 UTC
 
Comment 1 Yegor Kozlov 2005-07-11 15:30:22 UTC
Created attachment 15646 [details]
RecordFactory added to instanciate pp records
Comment 2 Yegor Kozlov 2005-07-11 15:52:16 UTC
Created attachment 15647 [details]
cvs diff for the previously attached jar

cvs diff for the previously attached jar
Comment 3 Nick Burch 2005-07-13 15:05:28 UTC
Is this an improvement on the new record instantiation code I committed on
Friday? It looks like a slight step back to me, since it has two places
recording the types, RecordType+RecordFactory, while my new code only has the
one (RecordType)
Comment 4 Yegor Kozlov 2005-07-13 15:18:38 UTC
(In reply to comment #3)
> Is this an improvement on the new record instantiation code I committed on
> Friday? It looks like a slight step back to me, since it has two places
> recording the types, RecordType+RecordFactory, while my new code only has the
> one (RecordType)

Yes, it is an improvement. We shouldn't put all eggs in one basket. RecordTypes 
should be simple. It's just a map of record ID and Name. RecordFactory is 
responsible for the new record instantiation. Such design keeps things simple 
and more object oriented. It is not a step back at all.
Comment 5 Nick Burch 2005-07-13 15:36:43 UTC
I consider it a step back, because there's a lot of duplication in your design.
Rather than have on class hold the triplet of name+type+class, you end up
duplicating information across two classes. It makes it harder to follow, and
increases the chances of mistakes/inconsistencies/confusion.

Personally, I find one long list in one place much simpler than two half
duplicated lists in two places...

It might not be purest OO, but I find it a more pragmatic design!
Comment 6 Yegor Kozlov 2005-07-13 15:51:40 UTC
(In reply to comment #5)
> I consider it a step back, because there's a lot of duplication in your 
design.
> Rather than have on class hold the triplet of name+type+class, you end up
> duplicating information across two classes. It makes it harder to follow, and
> increases the chances of mistakes/inconsistencies/confusion.
> Personally, I find one long list in one place much simpler than two half
> duplicated lists in two places...
> It might not be purest OO, but I find it a more pragmatic design!

There is no duplication. There are two objects and which of them is responsible 
for the specific functionality. RecordTypes is needed only to map ID and Name. 
That'all. Consider it as final class. It is not meant to be modified. 
RecordFactory is responsible for the instantiation. When you create a new class 
which extents Record you will have to register it there, not in RecordTypes! 

Also  I think we should follow object oriented design. It's going to be a big 
project and we must do it right. What seems pragmatic to you now can be a pain 
very soon!. 
Comment 7 Nick Burch 2005-07-13 16:45:20 UTC
It depends on if you consider things at just the java object level, or at the
overall object level.

There is a record "entity". For every kind of powerpoint record, we have an
"entity". This entity contains details of the type, the name, and the
implementing class. I don't want to duplicate any of that information, hence my
desire to put all the parts of that entity into one file.

Your scheme puts one pair (type+name) from the entity into on file, and another
(type+class) into another. There is duplication of "type" across two files, and
duplication isn't great.

I much prefere keeping the whole of the record's "entity" details in one class,
or failing that in just one file. If we did go down the two class route, I think
it would be via two auto-generated classes from one master meta-file, which
contained the tripplet of details for each record.
Comment 8 Yegor Kozlov 2011-06-24 10:01:57 UTC
Fixed long time ago.