Why change this? The reason why we set condition like this is readability. ! at the beginning may go unnoticed

A lot of the changes were rider auto applying my companies code guidlines. Youll see that in most files i touched. For some stuff like the m_ naming i changed project specific rider rules so it wouldnt change that too ;)

I don't get why you are setting all those to default string.Empty. The idea is that a field that is null has not been set while a field that is "empty" has been set to empty during parsing. null and string.Empty (or null and new string[0]) do have different meaning and I believe default should be not set (i.e. null)

this was because i initially set the project to use nullablility which then markes a string as an issue if not defined nullable explicitly or initilaized.
overall i just dont like Nulls in a public API; expecially in this case here a name is also the key. name would definitly be something to set in the constructor.
i generally way wondering why mostly no constructors are used. When i would use property initializers instead like that i would still set them to init for set: public string Name {get;init;} which works with the property initializers aswell

Given all the comments already provided about the read only approach I would stress that IEnumerable<T> is by design a readonly interface because it allows only reading (or iterating through iterator if you will), not editing (the collection).

Another change due to code guidlines wich i normally follow. We only use IEnumarbale if we have functions that yield return. IEnumerable for me markes that the list can be build up while processing which is not the case here

Ok, understood, sounds reasonable, event though how the collection is built, if lazy or eager, is a specific implementation detail of the implementing class.

I do get the intent here and I agree that precalculating some unmutable values (e.g. IsMultiplexed) may help with performance. Nevertheless I think this is the wrong place (I would not do this stuff in the constructor) and that from a design point of view there should be a better solution also to solve the read only part.

dbc being line by line requires mutability during parsing, this is why most objects were mutable. Properties are spread all over the dbc file and as such I need to keep an object open to add/change/override properties while I go. If the final object should be immutable and optimized, then probably it's a different object. We then need a internal class ParsingMessage (Message here is an example, but this should probably propagate to Signals too, if necessary) which is mutable (note the internal keyword, not public API) and when the parsing is over then ParsingMessages are converted into public class Message which is optimized and unmutable.
The conversion is not Message responsibility itself, but parsing engine's one (e.g. the parser knows whern it's done). Also, think about it, there is a sort of race condition. Message list is not ready/usable (IsMultiplexed is not telling the truth) until the list itself is passed into a dbc constructor...

Sure, that this would bring to some sort of code duplication but would allow better responsibility separation keeping mutability where required by language (i.e. dbc) design, forcing unmutable and optimized properties where required.

i would still think that using the immutable stuff would be good for public api. In fact the first implementation i did was using that.
I just had some problems with that: It was unclear if the packer should work on immutable stuff or not. i didnt like the classes exposed in the public api all being named immutable. Thats why i would name the immutable stuff just signal,message aso. and the mutable stuff that is internal could just be named mutable or something like that.
I also think the public api shouldnt allow reference replacement as then you could just add unvalidated stuff. thats why i did all the setter to internal.
And just wrapping the internal stuff with a readonly for pulic access meant that you dont have to rebuild the dbc necessaryly when only adding small stuff. With the immutable stuff you would need to regenaret the immutable objects.

I do agree that a read only dbc should be readonly.

The pattern we were using is found in other projects like google's protobuf (see: oneof). I personally do not like runtime type checking. Moreover, CustomPropertyDataType should be part of CustomPropertyDefinition so if you really want to keep the interface and type checking, maybe CustomPropertyDataType should be part of ICustomPropertyDefinition

the main problem for me with this approach is that you would always need a switch case when accessing the property which tends to be "long" code. In my code you coudl use the same thing for double,int and hex but for string and enum it still sucked ;)
the main thing that is annoying is that the property and the property defintion are seperated and you need this double checkign everywhere. I would consider it way nicer if you just have a custom proerty containing one of 5 objects which holds all the information in it.

Thanks for teh code review btw

Property and definition are separated beacause in the dbc they are separated, they are different tags.
This is a scenario where the parsing classes may require some trick but then for the API we can merge them together once the parsing is done

Cheers
A