Skip to content

Conversation

@KerstinKeller
Copy link
Contributor

@KerstinKeller KerstinKeller commented Nov 28, 2025

…ng types for better type safety.

Description

This is a hypothetical pull request that demonstrates how we could use strong types in eCAL with the example of the ProcessID.

It compiles when the library https://github.com/rollbear/strong_type is available in the include path, and ECAL_USE_STRONG_TYPES is changed to 1.

Some key findings:

  • Introducing strong types introduces coupling of parts of eCAL which had previously not been coupled. E.g. since the types are made availble as part of the public API, they are defined in the public headers, which then need to be included in some parts of eCAL which know nothing about the public headers.
    This can be mitigated, however, it might make sense that internal eCAL components define their own strong types, and we allow conversion between those strong types. (E.g. shm file handling is a small independent component, yet it also handles process IDS.).

  • We already need to convert at some boundaries: Serialization (duh!)

  • Making a library such as strong types part of our public interface is less than desirable. Which is kind of a bummer, and it would mean that we would have to convert to strong types after the dll interface boundaries.

  • There were a few structs which didn't default initialize the process_id. Using strong types enforced default initialization because otherwise the constructors would become deleted.

  • ProcessID is actually an unsigned long (DWORD) on Windows, not sure if we should change types or who made it int32_t.

  • I did not find any obvious bugs (e.g. entity id assigned to a process ID), but we did use some somewhat diverging datatypes in places (e.g. int vs int32_t).

  • It is alot of work to change a widely used entity such as the ProcessID.

#include <string>
#include <cstddef>
#include <ecal/config/configuration.h>
#include <ecal/types.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like the coupling we're introducing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants