ossrs / srs

SRS is a simple, high-efficiency, real-time media server supporting RTMP, WebRTC, HLS, HTTP-FLV, HTTP-TS, SRT, MPEG-DASH, and GB28181.
https://ossrs.io
MIT License
25.32k stars 5.33k forks source link

Should we declare the SrsConfig::get_?? methods as const? #3522

Closed suzp1984 closed 1 year ago

suzp1984 commented 1 year ago

https://github.com/ossrs/srs/blob/26aabe413daf348b9d41dbb5dcdfa72b6cf604a1/trunk/src/app/srs_app_config.hpp#L445

I think most of the get_?? methods inside SrsConfig should be declared as const:

virtual srs_utime_t get_threads_interval() const;

What do you guys think about it?

2nd question: https://github.com/ossrs/srs/blob/26aabe413daf348b9d41dbb5dcdfa72b6cf604a1/trunk/src/app/srs_app_config.hpp#L412

Should we also declare the return string as const? virtual const std::string get_pid_file() const;

winlinvip commented 1 year ago

For what?

suzp1984 commented 1 year ago

More C++ restrictions. The const method means its implementation can't doing any change to the object. This is helpful for further refactoring by other devs and cooperation.

winlinvip commented 1 year ago

We're graceful for your time and attention. However, we would like to avoid using too much programming language features , specially C++ template and const, as they can lead to significant complexity.

I also agree that the const feature is beneficial, as it can enhance the robustness and stability of a system. However, stability is also influenced by other factors, such as testing and utesting, which are currently the primary bottlenecks in SRS. It is crucial to prioritize the improvement of testing and unit testing before addressing other concerns.

In C++, the const keyword is used to indicate that a variable or object should not be modified after it has been initialized. This can help improve the robustness and stability of a system by preventing accidental changes to important data. There are several ways to use const in C++, and I'll provide examples for each.

The const feature in C++ is not a straightforward concept. The following answer is provided by GPT-4:

  1. const variables: When you declare a variable as const, it means that the value of the variable cannot be changed after initialization.
const int x = 10; // x is a constant integer with the value 10
// x = 20; // This would cause a compile-time error, as x is const and cannot be modified
  1. const pointers: When a pointer is declared as const, it means that the value stored at the memory location pointed to by the pointer cannot be modified.
int x = 10;
const int* ptr = &x; // ptr is a pointer to a constant integer
// *ptr = 20; // This would cause a compile-time error, as ptr points to a const int and cannot modify the value
  1. Pointers to const: When a pointer itself is declared as const, it means that the pointer cannot be modified to point to a different memory location.
int x = 10;
int y = 20;
int* const ptr = &x; // ptr is a constant pointer to an integer
// ptr = &y; // This would cause a compile-time error, as ptr is a const pointer and cannot be modified to point to a different location
  1. const member functions: When a member function of a class is declared as const, it means that the function cannot modify any non-static data members of the class.
class MyClass {
public:
    int x;

    int getValue() const {
        // x = 20; // This would cause a compile-time error, as getValue is a const member function and cannot modify x
        return x;
    }
};
  1. const reference parameters: When a function parameter is declared as a const reference, it means that the function cannot modify the value of the argument passed to it.
void myFunction(const int& x) {
    // x = 20; // This would cause a compile-time error, as x is a const reference and cannot be modified
}

Using const in these various ways can help ensure that your code is more robust and less prone to bugs caused by unintended modifications to data.

winlinvip commented 1 year ago

In summary, we don't currently need to use const, except in certain cases where performance is a concern, such as with the SrsContextId.