Bug 42161 - Identical code in LenyaVersion and VersionImpl
Summary: Identical code in LenyaVersion and VersionImpl
Status: NEW
Alias: None
Product: Lenya
Classification: Unclassified
Component: Miscellaneous (show other bugs)
Version: Trunk
Hardware: Other other
: P2 normal
Target Milestone: 2.0.1
Assignee: Lenya Developers
Depends on:
Reported: 2007-04-18 07:50 UTC by Richard Frovarp
Modified: 2007-07-16 03:08 UTC (History)
0 users

Removes VersionImpl (4.98 KB, patch)
2007-04-25 06:26 UTC, Richard Frovarp
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Frovarp 2007-04-18 07:50:48 UTC
LenyaVersion (src/java/org/apache/lenya/cms/workflow/LenyaVersion.java) and
are identical. LenyaVersion is used in
src/java/org/apache/lenya/cms/workflow/DocumentWorkflowable.java and VersionImpl
is used in
LenyaVersion or VersionImpl should be removed before the code freeze.
Comment 1 Andreas Hartmann 2007-04-25 02:37:27 UTC
That's not trivial (unless I'm missing an obvious solution). Since it doesn't
imply any functional problems, I'd say we defer it to 1.4.1.
Comment 2 J 2007-04-25 03:45:49 UTC
can you explain what makes this duplication necessary?
i can see how the core should not depend on a module, but i'm not sure about the
would moving the DocumentWorkflowable into a module help?
while we're at it: what other functionality should be modularized before 1.4?
should we maybe get rid of the old "core" entirely?
Comment 3 Richard Frovarp 2007-04-25 06:26:39 UTC
Created attachment 20037 [details]
Removes VersionImpl

This patch causes the code to use LenyaVersion instead of VersionImpl. Builds
and runs fine.
Comment 4 Richard Frovarp 2007-04-25 06:31:16 UTC
I'm not caught up enough to understand what exactly the src/java tree is. I
figure the modules should be able to depend on it. My patch is based on that
assumption. Alternatively, instead of removing a file that others may be using.
Have VersionImpl extend LenyaVersion and wipe out VersionImpl's code. This will
preserve the file, the methods presented, and operation while making any further
updates happen in one place. This might be the preferred route to get this into
the 1.4.0 release.
Comment 5 Andreas Hartmann 2007-04-25 06:36:15 UTC
Thanks for the patch!

But there's a little problem (which is the reason why I didn't make this change

(working copy)
@@ -20,6 +20,7 @@
 import java.util.ArrayList;
 import java.util.List;
+import org.apache.lenya.cms.workflow.LenyaVersion;

The code in org.apache.lenya.workflow is meant to be generic for SoC reasons,
i.e. it shouldn't know anything about the CMS. That's why it shouldn't import
anything from o.a.l.cms.workflow.

IMO a clean solution would be to use an abstract factory to generate version
objects. The o.a.l.workflow package uses


The o.a.l.cms.workflow package provides an implementation which generates
LenyaVersion objects.
Comment 6 J 2007-04-25 06:46:04 UTC
hmm. i see your point, but code duplication is the far greater evil imnsho.
how about taking richard's patch with a loud comment on how it should eventually
be refactored, and a low-priority enhancement bug?
Comment 7 Andreas Hartmann 2007-04-25 06:49:41 UTC
(In reply to comment #6)
> hmm. i see your point, but code duplication is the far greater evil imnsho.

Code duplication is not really evil. Knowledge duplication is, but IMO this is
not the case here.

> how about taking richard's patch with a loud comment on how it should
> eventually be refactored, and a low-priority enhancement bug?

I'd rather take the time to implement a factory, or think of a better approach.
Comment 8 Richard Frovarp 2007-04-25 06:51:09 UTC
Thanks for explaining. My confusion came from the fact that both files import
o.a.l.workflow.Version which is under the same file tree as
o.a.l.cms.workflow.LenyaVersion. While there might be a separation of code in
concept, I didn't see it reflected on the file system.