BRL-CAD is huge. With any large body of code, one inevitably ends up with a mix of good and bad coding practices. On the whole, BRL-CAD is actually better than most but we are constantly working on improving the code. This includes eliminating duplication.
References:
- http://en.wikipedia.org/wiki/Don't_repeat_yourself
- http://brlcad.org/wiki/Code_Cleanup
- http://brlcad.org/wiki/Compiling
- http://brlcad.org/wiki/SVN
This task involves reducing BRL-CAD's source code by 100 or more lines of code by refactoring and eliminating duplicate code. You can use whatever method you like to identify duplication, but beware that there are more than 1 million lines of code in BRL-CAD, so you're not likely going to find this duplication just by browsing.
We suggest using a code duplication detection tool like Simian. See our Code_Cleanup page for details.
Download our latest Subversion trunk sources and make sure you can compile cleanly first. Then you can run Simian or do whatever you need to find sources of code duplication. Make your edits, then make sure the code still compiles (run "make", "make test", "make regress" and "make benchmark" to test your changes). Finally, create and submit a patch file of your changes (see the references, svn will create the patch file for you).
Feel free to join the brlcad-devel mailing list or IRC channel to discuss your changes beforehand.
| File name/URL | File size | Date submitted | |
|---|---|---|---|
| changes.zip | 111.7 KB | December 17 2013 01:55 UTC | |
| changes.zip | 626.7 KB | December 18 2013 09:50 UTC | |
| patch.diff | 8.2 KB | December 20 2013 23:01 UTC | |
| duplicate_removal_3.diff | 57.9 KB | December 30 2013 16:31 UTC | |
| duplicate_removal_3.diff | 57.9 KB | December 30 2013 16:58 UTC |
I would like to work on this task.
This task has been assigned to Kristian Hansen. You have 72 hours to complete this task, good luck!
I try to download the repository of the BRL-CAD source code using TortiseSVN, and it requires me to input a username and password. Is the repository password protected?
Or is there a simple download link somewhere?
Refer this link
http://brlcad.org/wiki/Compiling#Download_BRL-CAD
I have ran a scanner for code duplication and there are more than 100+ lines of code duplicated.
I will remove as much as I can in the list before time expires, and all of the changes are included in a log as well as the changed files which I will upload later.
The work on this task is ready to be reviewed.
Kristian, I missed your earlier comment but wish I hadn't... you put in good effort but on code that is not ours! :) Everything in src/other belongs to other projects and is not ours to edit. Normally we tell Simian or any other parsing tools to ignore all of src/other (3rd party code), misc (infrastructure), and sometimes src/external (3rd party plugins). The duplication in our code is everything else.
Fortunately, you addressed more than 100+ lines in our sources as well (e.g., bigE.c) so for simplicity of working through this discussion let's focus there. Your modifications need to be submitted in patch format, which is how devs communicate changes to other devs.
Since you have an svn checkout, you can create a patch file fairly simply by running "svn diff mypatch.diff" in the top-level of your source tree and it'll capture everything you edited that is tracked by svn into a mypatch.diff file.
Before you run that, though, you should either run "svn revert -R src/other" (this will undo all of your edits in src/other) or be more specific with what you diff (svn diff src/libged my_changes.diff).
Submit that patch file and I'll give it a look over.
As an aside, related to your earlier comment -- we realize that there are more than 100+ lines of code duplicated. BRL-CAD is more than 1M lines and approximately 10% is "duplication" in some form. This is actually far better than industry average, but you literally could spend years trying to eliminate duplication.
For GCI, we just want you to get a taste and spend no more than a few hours per task claim of time coding. You can claim as many tasks as you like doing exactly this sort of reduction over and over if you demonstrate that you're doing it right. If you did that, you'd almost certainly make our top five and be a grand-prize contender.
The deadline of the task has been extended with 1 days and 12 hours.
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
I would release it in a patch format, but I downloaded it as a straight .zip from the link provided earlier.
Regarding doing work on your own code, is all of the work I have done in bigE enough? If not, I can locate and delete other code in the thing. Unfortunate that the work that I have done is moot now, if only you have told me earlier not to touch those folders.
Nethertheless, what is done is done, I cannot change it. I'll see what I can do though, but if I cannot get the SVN thing to work, I would just have to apply the patch in the form of a change source file.
Please respond to my previous question, don't want the same thing happening again.
The work on this task is ready to be reviewed.
I have finished the task, and done some more duplciation removal, a total of 308 lines. I was not able to submit it as a patch as I never grabbed the code from SVN or anything like that in the first place, it was provided to me as a link that was posted earlier in this message, you did not respond to that, so I had no other choice but to submit it as it is.
A log of all of the duplications detected is also included in the .zip folder incase you need it for future uses/reference.
Melange has detected that the deadline has passed and no more work can be submitted. The submitted work should be reviewed.
Kristian, you've done enough work to claim three tasks like this so far! Please ... stop increasing the scope of your work until we get just one right. :) Like I said earlier, just focus on bigE.c for now. That file alone is sufficient reduction. You can submit the other duplications you just worked on in another claim (we'll create as many code reduction tasks as needed).
Regarding your "if only you have told me earlier " comment, the changes you made in src/other are moot but we also had no way of knowing you were intending to put that much effort until you posted your files. Moreover, the Code Cleanup link we provide as a reference for Simian does mention only scanning our core code and the script we provide explicitly skips the src/other files. If only you had read the references... :/
Even though you didn't get an svn checkout, you can still create a patch file. You just diff the file unmodified against the file modified:
diff -u original.c modified.c your_patch.diff
Having a subversion checkout will be crucial for any future tasks, but you can just diff against the file you started with for this one.
Lastly, if you're interested, we have an IRC chat channel where mentors and other developers often hang out if you have questions. GCI task comments are responded to throughout the day, but IRC will usually get a response more quickly if you're patient, polite, and aware of the etiquette expected: http://brlcad.org/wiki/IRC
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
The deadline of the task has been extended with 2 days and 0 hours.
I find that 100 lines removed is too short for a task, so I strive to extend the task because it seems too easy just to remove around 100 lines of duplicate code.
I was also always taught that the more you do, the better your mark will be. I am determined to win, so I try to do as much as possible to impress my mentors.
The work on this task is ready to be reviewed.
Melange has detected that the deadline has passed and no more work can be submitted. The submitted work should be reviewed.
Don't worry about the tasks being too short, that's our job. :)
These tasks are short because removing dead code is actually a fair bit more complex than just deleting lines from files. Knowing which can and cannot be removed often takes time and careful effort.
That said, the file you submitted is not a valid patch file. The file is also not in universal format like I showed in my example (diff -u before after patch). Learning how to create and submit a proper patch file is also why I mentioned about just focusing on getting this first task right instead of trying to eliminate a lot of code. There's a whole process of identification, inspection, testing, and interactive review that is involved.
Patches are the language of devs, so this is one that's important to understand and get right, especially if you're going to be working on as many tasks as you suggest. If you have questions, just ask. ;)
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
The deadline of the task has been extended with 2 days and 0 hours.
Is there any special software that I need to input the (diff -u original modified patch) command, because it does not work in the command line prompt.
Please reply soon, before the task expires.
Melange has detected that the final deadline has passed and it has reopened the task.
I would like to work on this task.
This task has been assigned to Andromeda Galaxy. You have 72 hours to complete this task, good luck!
The work on this task is ready to be reviewed.
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
This is quite a massive reduction, but if you're going to create a new ged pointer, you also need to call ged_free(). Otherwise, you end up with a memory leak.
That said, have you also tested this change other than compiling? I'm not sure that it works because you only utilize the wdbp (the .g pointer) from the dgop (a display context). The "big E" command draws and the ged you created is not initialized for drawing. It's only initialized for database operations.
You can test your patch by running mged and archer. On both their command prompts with an empty database, you should be able to run these two commands:
make sph sph
E sph
I fixed the ged_free just now; E sph does seem to triangulate and draw the result in archer/mged; if I submit another diff, can you try it on your system and see if you see the same behavior?
The work on this task is ready to be reviewed.
Congratulations, this task has been completed successfully.
If it works for you (and you've actually tested it), that's usually good enough to commit a code change. We can always revert later if there's an issue, or inspect further. Carry on with the good work. ;)