Possible changes #1
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
indent -nbad -bap -nbc -bbo -hnl -br -brs -c33 -cd33 -ncdb -ce -ci4 -cli0 -d0 -di1 -nfc1 -i4 -ip0 -l80 -lp -npcs -nprs -npsl -sai -saf -saw -ncs -nsc -sob -nfca -cp33 -ss -ts4 -il1 -nut Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Add error checking and polish the style: - alphabet order of includes; - define default values. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Unify if-else style (use continue always); init loop variables right before the loop; unify spaces in comments. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
There is no point to duplicate buffer address to p variable, it's not changing, instead offset is used, remove it. Also sscanf is actually scans doubles (%lf), not integers, fix the comment. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Source data should be freed before overwriting. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
There is no big point to split the functionality into such tiny pieces: put all matrix specific operations into matrix.c module; put main processing into processing.c module (it is just 2 functions at the moment, read_file() and calculate()). No funtional changes. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Add set of methods to create, access and free matrix object. Also matrix_is_square() predicate. At the moment new/free doesn't really allocate the objects to be compatible with the current codebase. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Use wrappers to access matrix. Fix C++ comments. Change the signature -- there is no point to pass the size, the only user of the function passed .row field. Output error message to stdout. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Unify if-else-return. Change signature: remove n argument, there is no sense, it's a number of rows of the matrix. Use wrappers to access matrix object. Output error to stderr. More verbose message. Fix C++ comments. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Use wrappers to access object. Move memory initialization to matrix_new(). It slow down execution a bit and not necessary for all operations, but looks as a proper responsibility split. Fix C++ comments. Fix wrappers to respect the const modifier. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Will be used in add() check instead of direct access. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Use wrappers to access matrices. Output error message to stderr. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Use wrapper to access the matrix. Rename helper to print_elem. Remove extra blank lines. Use const modifier. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Use wrappers to access matrix. Change signature to take matrix as the first argument. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Use wrappers to access matrix. Free source matrix before overwriting to fix memory leak. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
All the operations return a new matrix and do not change arguments. That will make memory management for easier and consistent. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
That makes allocation and cleaning on the same level. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
There is no big point to allocate it dynamically, the size is known in advance. It can be better to make it static to save some stack, or change the logic for dynamic allocation but with automatic extending on demand. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
The overwritten objects must be freed. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Separate processing of file and creating of matrix. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Finish migrating to the public matrix API. Adds one more wrapper matrix_is_scalar(). Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
determninant returns double, but in the main code (processing) scalar value actually represented as 1x1 matrix. Return such a matrix instead of double, makes code more consistent. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
It's a pretty separate code, make a function for it. Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
data[count] is actually A[0][count], not [count][1] Signed-off-by: Yauheni Kaliuta <y.kaliuta@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi!
You asked some feedback on ##C channel.
This is something how I see it. I do not insist that it's a best way though. I tried to keep your logic untouched, sorry if did it.