> But are the patches not ordered in the wrong order? I mean, the first
> patch imports code which is only introduced in patch 5.
You're right. I'll send a new patch with the right order when I've
done further improvements based on your comments.
>> The code is by no means finished. Lots of TODO's are still there,
>> documentation can be improved, and I haven't done much to remove
>> trailing whitespaces, etc. However, I've reached a state where I'm
>> actually able to use it to run benchmarks on the DAIMI hosts while
>> data is reported to a MySQL database at my computer at home.
>
> Cool! I saw you write somewhere that an ORM would be nice and I agree.
> After having used such an interface I cringe when I see the raw SQL
> strings in the code -- there are just so many possibilities for
> putting MySQL specific stuff there, not to talk about the possible
> robustness and security problems.
>
> But we should not make that stop us from starting out like this.
No. I chose the second best solution, namely to isolate the SQL-stuff
in a single class. I actually had a look at using an ORM, but i
realized that it would take me quite some time to get used to. So I
did the usual SQL-stuff in order to get something going quickly. Not
the best motive, I admit. But I hope that we will be able to refactor
to ORM-stuff at a later time.
>
> I saw that you're using strings here and there as big comments:
>
> """bla bla, my nice
> explaination for
> something complex"""
>
> instead of the more natural
>
> # bla bla, my nice
> # explaination for
> # something complex
>
> It is only the very first string literal that is treated as a
> doccomment in Python -- the others are just string literals.
>
> Also, I saw you do
>
> if 'foo' in some_dict.keys():
>
> which is the same as
>
> if 'foo' in some_dict
>
> When you do
>
> str(benchmark.__class__).split('.')[-1]
>
> you could instead do
>
> benchmark.__class__.__name__
>
> which is still sort of a hack, but is easier to understand.
>
> Lastly, since the files are introduced this year, the copyright
> statements should simply read
>
> # Copyright 2008 VIFF Development Team.
Thanks. I'll wait for more corrections and then send a new updated
patch at some time.
If you want to play with the code and have problems getting it to run,
I'll be happy to help. Maybe the problems are due to some bug that I
haven't stumbled over yet.
Regards,
Thomas
_______________________________________________
viff-patches mailing list
[email protected]
http://lists.viff.dk/listinfo.cgi/viff-patches-viff.dk