Page 1 of 1

Code duplications and other bad practices

PostPosted: 15 Mar 2013, 11:00
by Max mtg
We've been discussing tracker that has not been used for months.

There are other tools in cardforge.org that have been abandoned and not used for a long time.

see for instance duplication detector module: http://cardforge.org/releases/site/cpd.html

There are also dependency listing modules like http://cardforge.org/releases/site/plug ... eport.html


Checkstyle report - http://cardforge.org/releases/site/checkstyle.html - I think its rules should be reconsidered to produce less errors.

And there's more to see.

Re: Code duplications and other bad practices

PostPosted: 15 Mar 2013, 21:07
by Max mtg
Code: Select all
forge/card/replacement/ReplacementEffect.java:267
forge/card/trigger/Trigger.java:385
Adressed by r20352

Re: Code duplications and other bad practices

PostPosted: 15 Mar 2013, 21:59
by myk
hey Max, could you possibly hold off on code cleanup until after I've merged my branch? I actually spent a good amount of time doing cleanup in there, and it would make things a little easier not to have to manage our merge conflicts.

Testing in there is coming along well, and it won't be too much longer before I can merge it.

Thanks for the pointer to the maven-generated code analysis, btw! It does really highlight the duplication problem we have.

Re: Code duplications and other bad practices

PostPosted: 16 Mar 2013, 07:50
by Max mtg
You've asked me to hold off any development in trunk, without even specifying which modules are problematic to merge for you.
That's not how the branches are supposed to work.

Re: Code duplications and other bad practices

PostPosted: 16 Mar 2013, 10:06
by Sloth
Don't bother too much with the AI ability classes Max. Some of the functions are just copy and pasted stubs from other abilities that are meant to be expanded/rewritten one day.

Re: Code duplications and other bad practices

PostPosted: 16 Mar 2013, 12:47
by Chris H.
myk wrote:hey Max, could you possibly hold off on code cleanup until after I've merged my branch? I actually spent a good amount of time doing cleanup in there, and it would make things a little easier not to have to manage our merge conflicts.

Testing in there is coming along well, and it won't be too much longer before I can merge it.
 
I think that you and I have ran about as many tests as we could come up with and it appears to be working really well at this time from what I can see.

I would suggest that the merge into the trunk be done this weekend if you have the time. This in turn will increase the number of testers that we have as those people who download the daily snapshots will then have a chance to explore this new feature.

Re: Code duplications and other bad practices

PostPosted: 16 Mar 2013, 12:55
by Max mtg
Will the merge have all decks and quests deleted?
Once I tried to switch to that branch and surprisingly lost all decks in res folder

Re: Code duplications and other bad practices

PostPosted: 16 Mar 2013, 13:38
by Chris H.
Max mtg wrote:Will the merge have all decks and quests deleted?
Once I tried to switch to that branch and surprisingly lost all decks in res folder
 
I ran a number of tests over the last few days with several different but recent builds from his branch and my decks were transferred and were not lost. Lets take a look, Constructed, Planar, Scheme and my quest data were transferred.

I had to reselect the deck in my quest file to continue my quest. But the quest data deck was there and my quest deck was listed, just had to double click on it to get it to the point where I would have opponents to face.

If you are curious you can try the most recent version at new branch build version here.

Re: Code duplications and other bad practices

PostPosted: 17 Mar 2013, 03:57
by myk
Max mtg wrote:Will the merge have all decks and quests deleted?
Once I tried to switch to that branch and surprisingly lost all decks in res folder
In merging, I see what you mean. Subversion is removing unversioned files instead of just removing the directory from version control. I'll add the (empty) directories back in to avoid the problem that you discovered. We can remove them again later after the people who update via subversion have all migrated their data. I apologize if the branch switch made you lose any data.

Re: Code duplications and other bad practices

PostPosted: 17 Mar 2013, 06:21
by Max mtg
Yes, this is what I meant. I used a secondary copy of sources to experiment with branches, so there was a primary copy to restore from.

Thank you for arranging this problem

Re: Code duplications and other bad practices

PostPosted: 17 Mar 2013, 06:36
by myk
I really do apologize. This was a side effect I did not anticipate. Most revision control systems will leave unversioned files untouched when their parent directories are removed form source control.

Re: Code duplications and other bad practices

PostPosted: 17 Mar 2013, 07:23
by Max mtg
Oops, looks like "arranging" is not a synonym to "fixing"... =(

I meant to say that despite of deletion of decks folder my personal losses were minimal due to presence of another copy.

Re: Code duplications and other bad practices

PostPosted: 17 Mar 2013, 07:38
by myk
haha : ) understood.