WIP Added Common Lisp implementation of Huffman Tree algorithm and of Quick Sort. #211
Conversation
tsandstr changed the title 
Gathros added the Implementation 
| Welcome to the AAA @tsandstr, and thank you for your contribution. I don't know Lisp (yet) but I can recognize beautiful code when I see it! Also, in general, we like the implementations to have some kind of example on how to run the code. Say, encode Speaking of quicksort, I haven't read it yet, but I need to tell you that we have no immediate plan on writing a chapter about that, so the code might sit in the fridge for a while. Thank you again for your contribution, it really peaked my interest in Lisp, and I feel I've already learned a lot by reading your code. |
| I can affirm, I loaded locally and was able to make trees of 2 or more distinct characters. Messages could be any length, so long as there were at least 2 leaves to make a tree. Here is a test file I used. It's not perfect. Download: huffmanTest.lisp.txt For those who may be curious or unsure on how to load a file into Common Lisp, one could update the path on the first line of the attached file, then use a similar command on a Common Lisp REPL to load the test file. One could also load only the huffman.lisp file this way, then test the functions using the REPL. CLISP is a popular implementation of Common Lisp which can be freely downloaded from their SourceForge. [CLISP homepage] The quick-sort also worked well for me, FWIW. I learned a lot by reading your code. Thank you very much for sharing, @tsandstr ! |
| Oops! I hadn't even though of creating a tree with just a single element in it. A weird edge case I suppose. I believe I have fixed that bug now. |
| @tsandstr Good to hear that you plan to contribute more code in the future! However, I suggest that create one PR for every algorithm you submit, so that the reviewing process is easier. And please stick to algorithms that are already in the Archive. Otherwise, we will have a ton of pull requests just sitting there, waiting for the chapters to be written and it will clutter up the list. |
| Alright will do. Do you have plans on adding more algorithms in the near future? And can community members write new chapters? …On Wed, Jul 4, 2018, 11:13 AM Marius Becker ***@***.***> wrote: @tsandstr <https://github.com/tsandstr> Good to hear that you plan to contribute more code in the future! However, I suggest that create one PR for every algorithm you submit, so that the reviewing process is easier. And please stick to algorithms that are already in the Archive. Otherwise, we will have a ton of pull requests just sitting there, waiting for the chapters to be written and it will clutter up the list. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#211 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFo7cpp6B-ArnLEvcJXStfxA6j7jlfN3ks5uDNuVgaJpZM4VBIW6> . |
| @tsandstr We're planning on adding more algorithms in the future. In fact the whole archive is a big work-in-progress. |
| Hey @tsandstr Thanks a bunch for the PR! Would it be possible to split this PR into 2 (one for huffman, and another for quick sort)? I might suggest creating a feature branch for both huffman encoding and quicksort so you can work on them independently and submitting both of those as separate PR's, but this can be done in a number of ways. The main problem is that quicksort is not yet in the AAA. It will be put in eventually, but sorting algorithms are kinda low priority because they are covered everywhere else pretty well and there isn't much for us to add to the discussion here. If you want to write a chapter on quicksort, feel free to do so and I'll thoroughly edit it to make sure it is clear and consistent. Please also talk to me (discord: https://discord.gg/Pr2E9S6) before you start writing because quicksort builds into a few other algorithms. If you do not want to write the chapter, I totally understand, but I would also ask for you to hold onto the quicksort code until we have it in the AAA. The code might need to change depending on what we need in the bulk text, so there might be modifications needed. Multiple PR's for algorithms we do not yet support encourage more people to submit code for algorithms we don't support and we'll start to get too many open PR's and no plan to merge them in the near future, so I want to avoid this case if possible. |
| It's been a week and I'm here to remind @tsandstr to split this PR up / remove QuickSort or we won't be able to merge it. |
| Sorry I haven't split it up yet. I am on a vacation right now and don't have access to my desktop computer, which is my only machine set up for this. I'll split it as soon as I can. Probably early next week. |
| Alright. Just wanted to see whether you're still here. |
jiegillet changed the title | @tsandstr How is the status? Do you still plan to work on this PR? |
| Hey @tsandstr, it's been a while since we had any discussion on this PR. Basically, we need it to be split up into 2 PR's and we need to reformat the huffman code to fit into the chapter, itself. If there is no response within a week, I might need to close this PR as it seems to be stale. |
| I think it's time to close this one @leios |
9 participants
No description provided.