Comments on SEOC2 Exercise 1 Part 1
Most of the comments I wanted to make applied to many people so I'll type
them once here instead of writing them on every submission.
Is this procedure sensible
Is the change I instructed you to make really an improvement? Only if the
extra complexity is going to be used. Remember XP's "simplest thing that
could possibly work". If you were following XP, you would not for example
introduce the abstract ButtonClient class until you had a definite need to
support several different ButtonClients - not just on the off-chance that
you might need to some day.
UML issues
Besides the basic mistakes (for which I took off marks mercilessly) the
exercise brought out some more interesting issues.
Note that UML interfaces cannot have attributes or outgoing
associations. The latter point is related to the dynamic/static
association point so it's slightly subtle, but this application wasn't too
subtle: the thing to note is that because an interface doesn't have any
data, in particular your abstract Button can't store a pointer to a
ButtonClient. So you needed an abstract class, not an interface.
An abstract class is shown using {abstract} (Using UML p88). It was also
fine not to indicate which classes were abstract: but if you did so you had
to do so in correct UML, so you lost a mark if you just copied the paper's
out of date version. I did generously allow << abstract >> as
this is what the first edition of Using UML had.
Note the difference between the realization arrow and the generalization
arrow.
Don't leave blank compartments in your class icons: it suggests that you
are listing the operations and attributes but there aren't any. If you
simply don't want to show that level of detail, omit the compartments
altogether and just use a labelled rectangle. (In fact, UML1.3 (perhaps
wrongly) states that empty compartments are never shown, e.g. if a
class has no attributes you omit the attribute compartment.)
Should we show attributes in UML that just implement the associations that
are also shown? Strictly, no; attributes should just be those things of
types or classes which are not shown in the diagram (e.g. because they're
too basic). However sometimes when the diagram is close to code, as here,
it's useful to do so. Just be aware whether you are doing so or not.
Order of refactorings
If you were puzzled about what the issue was here, it's probably because of
my mistake in setting the exercise. I intended to tell you that you were
not allowed to alter the Lamp code. If this has been the case, you would
have been forced to do the refactoring that added the adapter first. In
fact I failed to say this so almost everybody gave the refactoring order as
being the same as the way the variants were described in the paper. That
was fine, though I wrote comments about this on some people's work. You
only lost a mark if you said something along the lines of "finally if we
aren't able to alter the Lamp code we must..." - even though you'd already
suggested refactorings that wouldn't have been possible in that case - as
distinct from "if we want to be able to replace the Lamp by one whose code
we can't alter...".
The net effect was that it was easier to get high marks for this part of
the exercise than I really intended!
About code changes
The exercise specifically said you had to say what code changed, so, well,
for full marks you had to say what code changed. Brevity was definitely a
virtue, however. The best answers (most likely to be correct; easiest to
mark; easiest to work from, if I'd been a developer) were those that gave
code changes in a numbered list for each refactoring, with just a few words
per point. (E.g. "3. Replace Foo by Bar whereever it occurs in the code of
Mung").
I would have preferred to make actually altering some code part of those
exercise, but I couldn't because you don't share a common language
(basically CS4 speak Java and MSc speak C++). With hindsight I wonder
whether I should have given you some Java code and relied on Java being
similar enough to C++ (but simpler) that it would have been possible for
MSc students to pick it up. I'd welcome opinions on whether you think this
would have worked: I'm probably too close to both languages to trust my
opinion on this.
Testing
There was generally a very poor understanding of the role of testing in
refactoring: perhaps because you understood less about unit testing e.g.
from earlier years than I assumed. I hope this would be done better now as
we've considered testing again. The most (but not the only) relevant tests
are the unit tests: checks that the methods do the right thing. One of the
points about refactoring is that you change the unit tests as you change
the code, taking care to keep them effective. Of course some changes are
essential because the tests have to set up instances of the (perhaps
changed) classes in order to test them, etc. Many people blithely said that
there was no need to alter the tests, which isn't true in detail. I did
give you credit for saying sensible things about system tests as well,
though it wasn't what I'd hoped for. Many people said you should write new
implementations of the abstract classes you considered just in order to
check that the abstraction was sensible - OK in principal, but I think
utterly impractical. This, of course, is one reason for not inventing the
abstraction until you need it for real - you are more likely to be able to
get it right at the point when you need it, anyway, because you then do
have several implementations against which you can test it.
Miscellaneous
Be careful what you say about dependencies. Remember dependency is
transitive.
Notice that the ButtonImplementation's code must depend only on
ButtonClient, not on Lamp, otherwise the whole purpose is lost! In
particular, ButtonImplementation's constructor must take a ButtonClient,
not a Lamp. Of course it could still be passed a Lamp object at runtime,
because that's the beauty of inheritance polymorphism.
