Adding an SortedArray data structure #9
Conversation
|
Hi! Thanks for submitting this. I have a few comments. |
| #ifndef ALGORITHM_SORTEDARRAY_H | ||
| #define ALGORITHM_SORTEDARRAY_H | ||
|
|
||
| #include "arraylist.h" |
There was a problem hiding this comment.
A fundamental design decision of this library is that each pair of .c/.h files should be standalone. So it should be possible to take one of these files and just add it into a project without needing to depend on the entire library. Please refactor the code so that this is usable standalone without depending on arraylist.
|
Additional comment: the indentation style of your code is not consistent with the rest of the library. I use tabs for indentation, spaces for alignment. See here for details: |
|
Okay, I'll fix the comments. About the indentation: I suppose you mean I should use the third option from the blog article. |
|
Exactly - thanks! |
|
I have changed all the things you have commented. If there are still some problems, please let me know! |
|
This is neat. I will definitely be looking at the code to learn some things! Just as a note: I'm not sure if the maintainer of this project will care or not, but I wanted to let you know that you may want to do a git rebase before he merges this in to master. In doing so, you can squash/fixup some of your commits into other ones; for example, you could make it so that your code had the correct indentation style from the beginning, instead of being fixed in a later commit. That note aside, again, neat work. :) |
|
Thank you! I'll look into rebasing |
Included header, implementation and testing. Files have been added to Makefile.am Fixed sortedarray_index_of zero length failure. Removed -1 from initialising right edge. Refactoring: Replaced // comments with /* ... */ style comments Removed some debug code which was still present. Refactor: removed unnecessary function prototypes, removed full doc comments for static function Refactor: indentation style while keeping 80 flow limit. Refactoring, array syntax instead of pointers and some minor fixes. Rename variables to be clearer. Restructured tests around integers and fixed bugs detected by new tests.
|
Sorry I still haven't got back to you. I'll see if I can take a look today. |
| if (order > 0) { | ||
| left = index + 1; | ||
| } | ||
| else { |
There was a problem hiding this comment.
Should be on same line, ie.
} else {
| unsigned int i; | ||
| for (i = 1; i < sortedarray->length; i++) { | ||
| assert(int_compare( | ||
| sortedarray->data[i - 1], |
There was a problem hiding this comment.
Formatting is super weird here.
|
Thanks for reworking this - it looks much better! I noticed that the coverage analysis for the new file could be improved: Any chance we can get closer to 100%? |
|
Ping. |
|
I did not forget. I just currently have a lot things todo. |
|
Cool, thanks :) No pressure. |
|
I'll take a look at it |
|
Hey, I wasn't aware you had pushed new commits since you didn't comment. Feel free to reopen if you'd still like to merge this. |
|
Lol, next time I will give a ping. |
|
Thanks! |
|
:) |
Implements all meaningful operations of an array list for a sorted array list. Contains test code, documentation and source.