Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Prefer standard headers

#Prefer standard headers <bits/stdc++.h> isn't in any standard; even on those platforms where it exists, it's probably a poor choice if it includes the whole of the Standard Library. Instead, include (only) the standard library headers that declare the types that are required for the header.

Avoid using namespace in headers

#Avoid using namespace in headers AA header file should provide the definitions that the including program requires. If it brings all of std into the global namespace, that's a far-reaching side-effect that can make correct programs much harder to write.

Although there are differing opinions, I would extend this advice to program files too.

Namespace hygiene

#Namespace hygiene TheThe constants MAX_SIZE_ARRAY and MAX_SIZE_MATRIX would be better within their respective namespaces, rather than in the global namespace. I'm not entirely sure why we have those fixed limits at all, but that may be because I don't understand the real-world problem this code attempts to solve.

Interface design

#Interface design IsIs there a strong reason that std::vector should be the only supported container? And that float should be the only supported element type?

If not, then it's likely that the methods should be templated on those types. You may want to encapsulate the data type and methods into a class; alternatively (if you want to be more general), you might want to keep the procedural interface, but pass appropriate input and output iterators rather than containers.

Options to reduce duplication

#Options to reduce duplication MuchMuch of the matrix implementation is repeated application of the list implementation over the rows of the matrix. I think there's scope to simply call the list methods from within the matrix ones. It may well be worth moving the definitions out from the headers, leaving only the declarations in the headers (as usual for non-template code).

Allow output to any stream

#Allow output to any stream InsteadInstead of embedding the use of std::cout in display(), it's better to pass a std::ostream as parameter, to allow output where it's needed (e.g. standard error, file or socket).

If you create a class (or pair of classes), you'll probably want to write a stream operator:

std::ostream& operator<<() const

#Prefer standard headers <bits/stdc++.h> isn't in any standard; even on those platforms where it exists, it's probably a poor choice if it includes the whole of the Standard Library. Instead, include (only) the standard library headers that declare the types that are required for the header.

#Avoid using namespace in headers A header file should provide the definitions that the including program requires. If it brings all of std into the global namespace, that's a far-reaching side-effect that can make correct programs much harder to write.

Although there are differing opinions, I would extend this advice to program files too.

#Namespace hygiene The constants MAX_SIZE_ARRAY and MAX_SIZE_MATRIX would be better within their respective namespaces, rather than in the global namespace. I'm not entirely sure why we have those fixed limits at all, but that may be because I don't understand the real-world problem this code attempts to solve.

#Interface design Is there a strong reason that std::vector should be the only supported container? And that float should be the only supported element type?

If not, then it's likely that the methods should be templated on those types. You may want to encapsulate the data type and methods into a class; alternatively (if you want to be more general), you might want to keep the procedural interface, but pass appropriate input and output iterators rather than containers.

#Options to reduce duplication Much of the matrix implementation is repeated application of the list implementation over the rows of the matrix. I think there's scope to simply call the list methods from within the matrix ones. It may well be worth moving the definitions out from the headers, leaving only the declarations in the headers (as usual for non-template code).

#Allow output to any stream Instead of embedding the use of std::cout in display(), it's better to pass a std::ostream as parameter, to allow output where it's needed (e.g. standard error, file or socket).

If you create a class (or pair of classes), you'll probably want to write a stream operator:

std::ostream& operator<<() const

Prefer standard headers

<bits/stdc++.h> isn't in any standard; even on those platforms where it exists, it's probably a poor choice if it includes the whole of the Standard Library. Instead, include (only) the standard library headers that declare the types that are required for the header.

Avoid using namespace in headers

A header file should provide the definitions that the including program requires. If it brings all of std into the global namespace, that's a far-reaching side-effect that can make correct programs much harder to write.

Although there are differing opinions, I would extend this advice to program files too.

Namespace hygiene

The constants MAX_SIZE_ARRAY and MAX_SIZE_MATRIX would be better within their respective namespaces, rather than in the global namespace. I'm not entirely sure why we have those fixed limits at all, but that may be because I don't understand the real-world problem this code attempts to solve.

Interface design

Is there a strong reason that std::vector should be the only supported container? And that float should be the only supported element type?

If not, then it's likely that the methods should be templated on those types. You may want to encapsulate the data type and methods into a class; alternatively (if you want to be more general), you might want to keep the procedural interface, but pass appropriate input and output iterators rather than containers.

Options to reduce duplication

Much of the matrix implementation is repeated application of the list implementation over the rows of the matrix. I think there's scope to simply call the list methods from within the matrix ones. It may well be worth moving the definitions out from the headers, leaving only the declarations in the headers (as usual for non-template code).

Allow output to any stream

Instead of embedding the use of std::cout in display(), it's better to pass a std::ostream as parameter, to allow output where it's needed (e.g. standard error, file or socket).

If you create a class (or pair of classes), you'll probably want to write a stream operator:

std::ostream& operator<<() const
Source Link
Toby Speight
  • 87.5k
  • 14
  • 104
  • 322

#Prefer standard headers <bits/stdc++.h> isn't in any standard; even on those platforms where it exists, it's probably a poor choice if it includes the whole of the Standard Library. Instead, include (only) the standard library headers that declare the types that are required for the header.

#Avoid using namespace in headers A header file should provide the definitions that the including program requires. If it brings all of std into the global namespace, that's a far-reaching side-effect that can make correct programs much harder to write.

Although there are differing opinions, I would extend this advice to program files too.

#Namespace hygiene The constants MAX_SIZE_ARRAY and MAX_SIZE_MATRIX would be better within their respective namespaces, rather than in the global namespace. I'm not entirely sure why we have those fixed limits at all, but that may be because I don't understand the real-world problem this code attempts to solve.

#Interface design Is there a strong reason that std::vector should be the only supported container? And that float should be the only supported element type?

If not, then it's likely that the methods should be templated on those types. You may want to encapsulate the data type and methods into a class; alternatively (if you want to be more general), you might want to keep the procedural interface, but pass appropriate input and output iterators rather than containers.

#Options to reduce duplication Much of the matrix implementation is repeated application of the list implementation over the rows of the matrix. I think there's scope to simply call the list methods from within the matrix ones. It may well be worth moving the definitions out from the headers, leaving only the declarations in the headers (as usual for non-template code).

#Allow output to any stream Instead of embedding the use of std::cout in display(), it's better to pass a std::ostream as parameter, to allow output where it's needed (e.g. standard error, file or socket).

If you create a class (or pair of classes), you'll probably want to write a stream operator:

std::ostream& operator<<() const
lang-cpp

AltStyle によって変換されたページ (->オリジナル) /