search

MatthewKing / DeviceId

A simple library providing functionality to generate a 'device ID' that can be used to uniquely identify a computer.
MIT License
794 stars 118 forks source link

Use inheritance for specific DeviceIdBuilders #54

Open Fruchtzwerg94 opened 2 years ago

Fruchtzwerg94 commented 2 years ago

Hi,

I've noticed, that using general device ID features like the OS version can't be used device specific. This means, it is possible to call AddOsVersion not dependent on the system you are running on:

This works:

string deviceId = new DeviceIdBuilder()
    .AddMachineName()
    .AddOsVersion()
    .OnWindows(windows => windows
        .AddProcessorId())
    .OnLinux(linux => linux
        .AddCpuInfo())
    .ToString();

This works not: 

string deviceId = new DeviceIdBuilder()
    .AddMachineName()
    .OnWindows(windows => windows
        .AddProcessorId())
    .OnLinux(linux => linux
        .AddOsVersion()
        .AddCpuInfo())
    .ToString();

The reason seems to be some type specific handling here. From my point of view, this could be solved by changing the way to create the specific builders to inheritance.

If LinuxDeviceIdBuilder inherits from DeviceIdBuilder, overriding the corresponding methods, this would solve the problem. As an alternative the base builder property could be made public. Is there a reason why this is not done like this? If not, I could create a PR with the proposed changes. Let me know what you think about it.

MatthewKing commented 2 years ago

Hello. Sorry for taking so long to have a look at this. This definitely looks like a reasonable suggestion. I will have a look into it. Alternatively, if you're happy to create a PR then that would be great. Cheers.