On 07/12/2018 01:25 AM, Emmanuel Lécharny wrote:
It's almost a 10 years old code (OMG, I can't believe I wrote that
already 9 years ago...), and my guess is that when I added the list of
throwable, I forgot to remove the other method, which hasn't eveolved
since then, which explain why the code is so different.

I thi,k we are safe if you simply remove the method you have renamed...

Done. Gone. :-)


2) Ava. I think Ava is supposed to be a simple object, almost like a
DTO.
Not sure what you mean by DTO in this context... Because it's serializable ?

Ava is supposed to be (almost) simple data class without (much) dependencies on anything else in the API. At least that's how I understand it. But I also understand the need for this to be (somehow) schema-aware and currently I do not see any easy solution how to properly refactor Ava. Anyway, I'm quite OK with current solution. The logging statements in constructors were polluting logfiles during AD schema parsing. But as they are now removed I'm good.

I have also found potential issues with Ava
comparison/normalization .... I have put a comment in the source code.
This is probably something to have a look in the future.
Actually, it's the equals() method, and in the comparison done when we
have a SchemaManager, we have to assume the value of both Ava has
already been normalized. This try-catch is a bit superfluous, we most
certainly can get rid of it...

No problem with the current implementation. The error handling just looked strange and suspicious. I remember I had some issues at this very point with semi-schema-aware LDAPs like AD. But I was able to work around it. So we are good with current code.

Good job. Have you tried to build the server with those changes ? If
not, I will do that tomorrow evening.

No, I haven't tried the server nor studio.

Thanks Radovan !

And thank you for help and patience with me.

--
Radovan Semancik
Software Architect
evolveum.com

Reply via email to