Looking at the code

Started by mercurialuser, March 11, 2015, 11:35:36 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

mercurialuser

I've discovered FET last saturday and I already spent several hours studying its code.
I compiled on both Windows and Linux.

I don't know how the project code is managed so I ask... 

To better study the code I'd like to clean generate.cpp:
- removing all the commented code
- adding comments to illustrate the algo
- check to see if I can use some macros for code
- in randomswap try to remove all that ok* variables that can be reduced to a couple (since they are not used outside their block)
- remove the #if 1/#endif and if(1) { ... }  blocks

I repeat: I don't know how the code is managed and why it is in this state, perhaps there is a reason. So I ask if you are interested to have the changes back or not.

Of course, I will also ask some questions on the algo when I'll be stuck... :-)

Liviu Lalescu

#1
The reason the code is untidy is that it was designed on the go, without proper design.

Thank you for your offer! But I would prefer for now to keep it like it is, because any changes are critical, and also I would prefer to see even the old code.

About macros - beware! These are very tricky.

I will try to help you, answering the questions you might have, as possible.

I will add your words in the TODO file.

mercurialuser

Hi Liviu,
I perfectly understand your words and I don't want to change your ideas!

I'd just like to report some thing that may give a little speedup to the code and to avoid duplicates.

For example in ::generate several matrixes are reset first and then assigned. The exact same code is used for loading teachersTimetable and newTeachersTimetable but they could be compacted in one loop. There may be probably less cache hits for accessing these 2 matrixes but I think that the multiple loops and retrieving act variable multiple times are more cpu expensive anyway.

For assigning these matrixes and some others the code loops (there are 2 variants with different variable names):
for(int j=0; j<gt.rules.nInternalActivities/*added_act*/; j++){

The code in the loop does some checks that are replicated in several other loops but they check on data that, as far as I understand, can't change between consecutive checks. So it seems to me that they are redundant.


Liviu Lalescu

Thank you for your work!

Again, I added your words in the TODO file. I think you are right, I checked a bit. However, I am very afraid to introduce new bugs to the code. But I might consider this in the future, if I will make some more changes to generate.cpp and testing.

There are very many places which is possible to improve.