[task #15317] Concatenate two or more tables

[Mohammad Akhlaghi]

Follow-up Comment #15, task #15317 (project gnuastro):

Thanks, its getting much better ;-). I can do all of these myself (and it
would take much less time for me than writing these comments), but I think it
is much better to get you to a good start now. This way, your future projects
in Gnuastro will be much more smoothly merged :-D.

Here are some points that occurred to me:

0 The commit message description is very good now, thanks! But length of the
lines in the commit message make it very hard to read. Take a look at some
previous commit messages: all the lines have roughly the same length. Also,
the last one is too long (it shouldn't be more than 75 characters).
0 The name of `concat_along_row' doesn't follow the standard and is confusing:
since its in a file called `table.c' it should start with `table_'. Also,
since its job is concatenate columns after each other, having the name `row'
can be confusing. Its good to give it a very similar name to the option.
0 In `concat_along_row', you can use the `gal_list_data_last' function (in
list.h
<https://www.gnu.org/software/gnuastro/manual/html_node/List-of-gal_005fdata_005ft.html>)
to avoid the while loop and help in readability.
0 The function and loop braces (`{' and `}') don't follow the GNU style
<https://en.wikipedia.org/wiki/Indentation_style#GNU_style>. Please have a
look at the other functions/loops to get a good feeling ;-).
0 Since this option is about `c'olumns, maybe setting `UI_KEY_CATCOLUMN' to
`C' is a better and more clear choice for `--catcolumn' compared to `f', what
do you think?
0 You can just define `catcolumntable' in `concat_along_row', not in the main
program structure. You can simply pass it the name of the file, and it can
read into memory there. Such higher-level input files can be read outside of
`ui.c'. This will also be easier later when you add row-concatenation, and
also later when you allow many tables. Also, you have added the sanity check
in `ui_preparations', but the check should be in `concat_along_row' (when the
input table's final set of rows is found).

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/task/?15317>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/