UAuthority.device() - empty or not?!

[AnotherDaniel]

I am currently working to port the java sdk to Rust, and am unhappy about how device and domain member fields are treated in UAuthority.device() / .domain():

The class happily accepts and stores empty and blank strings for these fields - but then, in the getters, it returns an empty Option in case they are empty or blank. I feel this to be quite misleading in general terms, plus it is not at all nice to transport into idiomatic Rust - where member fields are usually accessed directly, without indirection through getters. So there I have to decide whether I'll treat empty strings as 'there' or 'not there', but cannot elegantly discriminate between 'actually they are there, but if you ask for them I'll pretend they aren't...'.

So I would suggest to decide what UAuthority wants to do with these fields - do empty/blank strings count as values, or do they not?

(in the Rust SDK, I'll for now implement a 'blank/empty don't count' behaviour - but that breaks a bunch of test cases that I'm trying to keep in line with the Java versions...)

Cheers, Daniel

[stevenhartley]

Hi @AnotherDaniel , Domain is optional but device is not when using long form URIs (see recent updates for long vs short vs micro URIs) so an empty domain is a valid domain. The java SDK splits up the data model from the validators of the data model whereby the validators would return status if a URI is valid or not per a specific format (long/short/micro). Steven.

[AnotherDaniel]

Hm, I don't think I would argue against a logic split between model and validator - I'm just looking at the UAuthority class, where:

• the member fields can contain " " (fine with me)
• the getter() method actually messes with the value, by returning an empty Option even if the actual field is " " (this I very much do not like)

I'm not that fluent in Java anymore, it's been a few decades, but I'm thinking that:

• if the actual field is private, and cannot be accessed without the getter
• and the getter by default manipulates the field's value in certain cases
• why not directly "fix" the desired value at construction? Like: if domain String is empty or blank, then we leave that member empty (null)?

Yes, that would result a follow-on change in toString(), but I'd be in any case very confused if a getter gives me a different value than the pretty-printer...

[tamarafischer]

Hi @AnotherDaniel , The internal structure of UAuthority and how you want to hold the fields is up to the SDK developer. If you want to hold null values, that is fine. The getters on the class are the API of UAuthority. By definition, UAuthority can have a device and a domain. Using the Java Optional class lets us indicate if a value exists or not - it is the API. Using Optional we are clearly specifying that this field may not exist and we are doing it in a nice functional way since you can't really do anything with null and you have to check for it everywhere. Validation code is easier and we can define default values or any other operation that is available on the Optional since it is an object and not null, that like I said, can't be composed and is not very nice to work with. We make sure we always return a value of a specific type. Just a quick google for "the billion dollar mistake" is the main reason why I prefer to code clear interfaces that return a value and don't lie (throw exceptions) This is just me trying to take some functional programming concepts to the SDK. I am no Rust expert to say the least, I am a true novice starting my journey. They say that it is like Scala so I should be OK.

I would be glad to discuss this with you if you still find this a big problem in Rust.

Cheers, Tamara.

[AnotherDaniel]

Hm, somehow I'm failing to make my point, I think.

Just to get that out of the way: do not have an issue in the Rust implementation - I've opted to clearly distinguish between "value exists" and "value doesn't exist", and Options are an idiomatic thing in Rust anyways so I'm using them all over.

I'm trying to point out that the current implementation of UAuthority has an internal inconsistency which shines through at API level, and which I believe is not necessary, nor pretty. As in:

1. Create a UAuthority object with " " as a domain (note: this is two whitespaces, although doesn't really matter in this specific case).
2. call authority.toString() on that object, and you will get something that tells you it's got a domain of value " "
3. call authority.domain() on the object, and it will tell no "no, actually I'm empty, nothing to see here" (4. the equals() method btw also compares the " " values, not the Empty value that the class tells you it has, if you ask it via domain())

Might be that this is a normal pattern in Java, I really don't know. But it threw me when I figured out why one of the tests that I ported over wasn't behaving as expected...

[tamarafischer]

Hm, my time to think. I will have a closer look at the code in the morning (Timezone UTC+3) and get back to you. In general, the getters are the external API, toString is for debugging and should not be used as an API and equals and hashcode are also mostly used internally (if you want keys in hashmaps for example) In Java 17 I dont even need to explicitly add them anymore. I needed the SDK to be Java 11 compatible because of current clients.

Will get back to you.

Thanks, Tamara.

[AnotherDaniel]

Hi Tamara, to illustrate my point, consider this test case:

    @Test
    public void test_equal_not_equal() {
        UAuthority lots = UAuthority.remote("   ", "");
        UAuthority many = UAuthority.remote("    ", "");

        assertEquals(lots.device(), many.device());
        assertEquals(lots.domain(), many.domain());
        assertEquals(lots.address(), many.address());

        assertEquals(lots, many);
    }

The last assertEquals currently fails, while the first 3 succeed. Just does not feel right.

The reason is simply that the device() and domain() getters currently conditionally modify their return value:

public Optional<String> device() {
    return device == null || device.isBlank() ? Optional.empty() : Optional.of(device);
}

Instead of doing the isBlank() check (and subsequent Optional.empty() return) here, I'd suggest performing this check in the UAuthority constructor, and there deciding that the device and domain fields should actually be.

[tamarafischer]

Hi Daniel, I understand. Equals and Hashcode work with the raw values. The object lots and many are not equal. Their raw values are not the same. The raw values are exposed via the toString method and the equals/hashcode methods.

device(), domain() and address() are business logic API They take the raw values and return a business answer as an Optional if the raw value is actually useful in the business sense.

A device/domain or address that contains only spaces, in the business sense, is empty even though the raw values are not entirely the same. I was aiming at data structures that are not not anemic - meaning just holding values, but have a little business value inside that helps with their usage. We have been using the data structures for a while and it evolved with usages that we found in Android and in Cloud Java uE implementations.

We tried not to change the raw values, but instead provide API that makes sense business wise.

That said, I do understand that the values could be fixed in the constructor, but on the other hand we did not see the need and the API provided for the use cases of taking Strings from the transport layer (source and sink) and making objects out of those strings - with cleaner API - to make it easier to work with. The constructor would be doing work to satisfy a test. The values you provided are not the same, but business wise they have the same meaning. I think this is the main value of an SDK - providing value for uE software.

[stevenhartley]

Can we close this issue @AnotherDaniel after looking at the changes on the utransport branch?

[AnotherDaniel]

Looking good to me, now!