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

  • From: Fabian Gundlach <fabian.gundlach@xxxxxxxxx>
  • To: contestms-dev@xxxxxxxxxxxxx
  • Date: Wed, 13 Nov 2013 11:12:25 -0500

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: