Overload Read/Write to actually use Value directly

[jkoplo]

@timyhac - This is what I was talking about in chat. When testing in real-world I found myself with a lot of lines that looked like:

workOrderRequest.Value = workOrderDTO.Request;
workOrderRequest.Write();

workOrderQuantity.Value = workOrderDTO.Quantity;
workOrderQuantity.Write();

workOrderDate.Value = workOrderDTO.Date;
workOrderDate.Write();

It felt really redundant for something that is a pretty common need. The PLC side was written by an external supplier, so remapping into a UDT for a single write wasn't really an option.

The overloads should be a non-breaking API change (not that it matters at this point). You can stick with the old functionality above if you need to queue a bunch of writes, or utilize the overload to pass values as part of the Write:

workOrderRequest.Write(workOrderDTO.Request);
workOrderQuantity.Write(workOrderDTO.Quantity);
workOrderDate.Write(workOrderDTO.Date);

Thoughts? I don't really see a downside other than having more API calls and maybe being a little more confusing to people using the library since there's options?

[timyhac]

@jkoplo - so I've just been writing a HMI for an application and ended up implementing a void Write(T value) and T Read().

Although I would always recommend that a consumer of LibPlcTag writes their own wrapper (so that they can do testing), I also see the utility in having the overloads you recommend, so I'll admit I was wrong to doubt this feature change - sorry!

[jkoplo]

Ha! It's really a matter of convenience and honestly a lot of new c# devs get confused by generics.

I think it depends on our target end users. For someone who's mostly an AB programmer and is writing their first C# app, I think the having the read/write functions in library might be nice. OTOH maybe that's too many choices and gets confusing.

Personally, I'd like to see them in library. I'm going to implement them in every project I do, and to me that's the definition of something that belongs in a library.

[jkoplo]

@timyhac - This has sat out there for a long time... How do you feel about it?

If we're going to rev the minor for additions to the API this might be worth adding. I still think it has value.

[timyhac]

Yep 👍 - are you ok to update the PR?

[jkoplo]

For sure. I'll include some README updates too.

[jkoplo]

@timyhac I finally tidied this up and made it ready. This should be non-breaking, though technically the changes to ITag could (maybe) affect someone. I decided not to add anything to ITag around Write(object value) because it's one thing cast a type to a generic object and a whole different thing to cast an object to a specific type.

[timyhac]

Do we need ITag? I don't get why its there.
If we do, we could make it generic and add Write(T value) rather than Write(object value)

[jkoplo]

"Need" is a strong word, but yeah, I want it. The scenario is that you have a list of ITags (of different types) and you want to do a Read on the whole list for performance/packing reasons. Or a Write (after .Value had been set individually).

Making it a generic means it's no longer a common interface for all Tags. We could also have ITag<type>, but it would be for other reasons.

There's also some implications for mock or for offline testing for downstream projects.

[jkoplo]

Alright I snuck in another commit with 'Value' in the ITag. I've got a project that needs this, but it's a bit of an advanced scenario. I think this makes sense to roll out as a pre-release @ 1.1.0 since previous versions won't necessarily be compatible with projects built against this.

If you could review, merge, and publish I'd appreciate it.

[timyhac]

Yep alrighty - if we could add both ITag and ITag<T> on the Tag class then the consumer could use either generic or non-generic interface as they see fit.

var tag = new Tag<int>();

ITag<int> generic = tag;
generic.Value = "asdf";     // doesn't compile

ITag nonGeneric = tag;
nonGeneric.Value = "asdf";  // does compile

interface ITag
{
    object Value { get; set; }
}

interface ITag<T> : ITag
{
    new T Value { get; set; }
}

class Tag<T> : ITag<T>
{
    public T Value { get; set; }
    object ITag.Value { get; set; }
}

[timyhac]

I realize now this was already merged/released - all good.