[contestms-dev] Re: Unify submissions and user tests

  • From: Luca Wehrstedt <luca.wehrstedt@xxxxxxxxx>
  • To: Fabian Gundlach <fabian.gundlach@xxxxxxxxx>
  • Date: Thu, 14 Nov 2013 19:11:48 +0100

Ok, I did never use mixins and therefore never needed @declared_attr. I
hope Giovanni will be able to answer you on other matters, as he's probably
the one that has the clearest picture of the current situation.

Luca


On Wed, Nov 13, 2013 at 5:12 PM, Fabian Gundlach
<fabian.gundlach@xxxxxxxxx>wrote:

> Hi,
>
> > are you sure all these "@declared_attr"s are really needed?
> I'm completely new to SQLAlchemy, but when I omitted @declared_attr, I kept
> getting error messages saying that I can't use ForeignKey or relationship
> without @declared_attr, so I obeyed :). The documentation says so, too:
>
> http://docs.sqlalchemy.org/en/rel_0_8/orm/extensions/declarative.html#declarative-mixins
>
> About smart vs. dumb, I don't really have a clear opinion. But it seems to
> me,
> that in most places distinguishing between submissions and user tests is
> simply not necessary (if we rename the fields). How about just using
> isinstance(...) checks in the few places where it is? I didn't read the
> whole
> ES code, though...
>
> > PS: Fabian, we are often asked to authorize your emails to the ML because
> > it seems you're not subscribed. Could you check it? Thanks.
> Aha! I was already wondering why the mailing list was so slow, in
> particular
> during the night in Italy ;). I used different mail addresses for sending
> and
> for receiving mails. Should be ok now, I hope.
>
> Best,
> Fabian
>
> > I've nothing against using common base classes, renaming fields to make
> > them equal (I prefer parent_ to abstractsubmission_) and introducing a
> > "UserTestEvaluation". Yet, since I'm not an expert of databases and of
> > SQLAlchemy's ORM I cannot give actual coding suggestions. Nevertheless I
> > gave a quick glance at your code: are you sure all these
> "@declared_attr"s
> > are really needed?
> >
> > As for Giovanni's proposal, I don't like "smart" database entities
> because
> > I don't like them to mess with higher level concepts, i.e. they should be
> > as unaware as possible of how they will be used, of the workflow of the
> > evaluation, etc. That's because I'd prefer to have higher levels build on
> > lower ones, while keeping them clearly separated, instead of mixing and
> > tangling them together. This, IMO, helps clarity, debuggability,
> > reusability and modularity.
> >
> > Luca
> >
> > PS: Fabian, we are often asked to authorize your emails to the ML because
> > it seems you're not subscribed. Could you check it? Thanks.
> >
> >
> > On Tue, Nov 12, 2013 at 9:35 PM, Giovanni Mascellani <
> >
> > mascellani@xxxxxxxxxxxxxxxxxxxx> wrote:
> > > Hi.
> > >
> > > Il 12/11/2013 19:56, Fabian Gundlach ha scritto:
> > > > an easy way to reduce code duplication would be to write common base
> > >
> > > classes
> > >
> > > > for Submission-UserTest, File-UserTestFile,
> > >
> > > SubmissionResult-UserTestResult,
> > >
> > > > Executable-UserTestExecutable. I've tried this out here:
> > > > https://github.com/fagu/cms/tree/abstractsubmission
> > > > It doesn't require any changes to the database and already reduced
> the
> > >
> > > code
> > >
> > > > size by 178 lines :).
> > > >
> > > > A problem is that some fields with similar meaning have different
> names,
> > >
> > > e.g.
> > >
> > > > submission_id vs. user_test_id, submission vs. user_test,
> > >
> > > submission_result
> > >
> > > > vs. user_test_result. This seems to be quiet a nuisance in ES (some
> > >
> > > functions
> > >
> > > > are almost identical apart from variable names). Maybe we could just
> > >
> > > call all
> > >
> > > > of them submission_...? If we don't want to change the database
> column
> > >
> > > names,
> > >
> > > > we should at least create "aliases" abstractsubmission_... pointing
> to
> > > > submission_... or user_test_..., I think.
> > > >
> > > > Another thing we might consider is creating a counterpart to
> Evaluation
> > >
> > > for
> > >
> > > > user tests (instead of putting the corresponding columns into
> > >
> > > UserTestResult).
> > >
> > > I don't have much time to think about that now, unfortunately, so my
> > > contribution is a little vague for now. I think that changing the
> > > approach may help us: in particular, I'd evaluate the possibility of
> > > using "smart" objects, instead of using them just as C structs (Luca,
> > > Stefano and I already had a discussion about this and, if I remember
> > > correctly, Luca was not very enthusiastic about this proposal).
> > >
> > > Making Submissions and related classes "smart" instead of "dumb", as
> > > they're now, should help fixing their interaction with other objects
> and
> > > reusing code. For example, ES shouldn't analyze the object to determine
> > > its priority in the queue or build its associated Job object. It should
> > > just call some opaque method which would do anything the right way
> > > directly. This way you don't have to worry about internal differences
> > > between Submission and UserTest, as long as they both offer an
> interface
> > > which exposes the relevant calls. Same thing for the Job generation.
> > >
> > > Did you think about this approach? What do you think about it?
> > >
> > > Gio.
> > > --
> > > Giovanni Mascellani <mascellani@xxxxxxxxxxxxxxxxxxxx>
> > > Pisa, Italy
> > >
> > > Web: http://poisson.phc.unipi.it/~mascellani
> > > Jabber: g.mascellani@xxxxxxxxxx / giovanni@xxxxxxxxxxxxxxxxxxxx
>

Other related posts: