Great job. +1 Jinlin Hong
Sebastian Rühl <sru...@apache.org> 於 2024年9月27日 週五 下午3:07寫道： > Awesome work, sounds good to me. > > +1 > > (although about the List type I have to think a bit more... :D) > > - Sebastian > > On 2024/09/27 06:37:32 Christofer Dutz wrote: > > Hi all, > > > > so I have been spending a lot more time than I wanted to on cleaning up > in the PlcValues and especially the PlcValueHandler. > > > > Here I’ve noticed a few things: > > > > * We had some PlcValues extens the IECValue type and some a > SimplePlcValue type … I updated all to have the same base-type > > * Now all PlcValue types have an “of” method and constructors > accepting the major types: Byte, Short, Integer, Long, Float, Double, > BigInteger, BigDecimal, String (and some also specialized ones like: > Boolean, Duration, LocalTime, LocalDate, LocalDateTime. > > > > In order to allow creating almost any PlcValue with almost any Java > type, for the temporal types I had to decide which “encoding” the numeric > values are in. In general did I allow floating-point values, which are > simply treated as integers (ignoring the fractional part). However this > unified approach makes using them easier, but it makes less use of the > possible value range. While a long is able to produce any sensible date, a > byte can only reference dates on the first day of 1970. > > > > Now we could use different measures depending on the type, but I think > that makes things a lot more complicated. If a PlcDATE for example would > use “byte” as days of the current year (we could somehow use it a bit, but > it wouldn’t be able to represent a full year), a short could be something > different … > > > > I would say, we stick to one unified measure for all numeric > constructors per type and if somone thinks it’s a good idea to represent a > date with a byte, that’s his problem to solve. Otherwise, I could imagine a > complete cascade of possible problems: Someone has an existing PLC4X > program and notices that one variable, which was a short, now requires an > integer … as soon as he changes that the code stays compatible, but now the > conversion of value-to-plcvalue uses a different measure. > > > > For allowing to support different measures for the temporal fields, I > would much more suggest to use speaking constructor methods (as we have > been doing) … something like: > > > > PlcLTIME.ofNannoseconds > > PlcLTIME_OF_DAY.ofNannosecondsSinceMidnight > > PlcDATE.ofSecondsSinceEpoch > > PlcDATE.ofDaysSinceEpoch > > … > > > > And one last thing … I would like to remove the LIST type in > PlcValueTypes as this is actually not a type, it’s more an implementation > detail of how we implement array types. > > > > What do you think? > > > > As discussions on the Plc4x mailing list have become sort of monolouges > again, till I hear any real objection, I’ll continue my path (However if > you support me, leaving a +1 lets me feel less like I’m wasting my time > even trying to discuss things) > > > > > > Chris > > >
