Thanks for the great feedback. In my case, if for whatever reason, I am not able to parse response or execute URL properly, then I am returning empty List back. I guess that's ok in my use case here. Also to execute URL, I am using AsyncHttpClient which executes URL and return whole JSON response back as a String. And since executing URL can give some IOException or `TimeoutException', didn't got a response within a particular time, so that is why I return Optional<String> back as well.

hmmm so what I am supposed to do in PartialResultException constructor? Also I am not sure under what cases NoSuchElementException exception will be thrown. In my case, I can throw IOException since I am executing URL and that is what the exception can get thrown in that method.

You negate the two most important annotations as I ordered them by importance. The other things in comparison are trivial while the first two things were very hard to unpack as they have interdependencies and semantic issues. As your comment came while writing this comment I will extend my answer.

I also edited my question to add execute method which executes the URL.. Just to give you some idea how it looks like.. In my case, I am thinking to retry execute method few times if it throws exception. Because I want to get the data back at any cost by executing these url's. So for whatever reason first time, it gave IOException or Timedout for whatever reason, I can try calling execute method few times to make sure it give the response back.

Try to call the method a few time is the responsibility of the execute()-method. The assumption is that this method returns a proper value. If it cannot for any reason you have to throw an exception. This exception is wrapped within getIds to map it to an exception of your domain with additional information. Finally the caller-method can decide what to do.

hey saw the edit and I understood now.. So if I have to retry execute method say 3 times, how can I do that now since it throws RuntimeException now in the catch block so I might not be able to retry anymore? right? or I missed anything?

Also in many cases, next boolean will be false

but key next will be present there but it will be false so that means there is no another set of url at the bottom

hasMore = jobject.has(KEY_NEXT);

It is up to the strategy you want to have in your execute-method. But you have to be clear about its semantics.

what will has do here? it will check whether field is there in the json but it wont check whether it is true or false?

yeah that I know but with the current way execute has, you mean to say I should throw at the last retry attempt then?

I see you have TWO next-values in ONE object.

next-kexs

I am talking about this

"next": true,

most of the time with the first json response, it will be false and that means there is no url at the bottom of the json..

but some times it will be true, so there is another url at the bottom which I need to use to get annother set of ids and keep repeating

"has" will check the existence of the field, not the content.

and in your code I see this line

hasMore = jobject.has(KEY_NEXT);

yeah thats what I thought

My assumption was that next is a unqiue key.

has will check the field so it will always return true

and then it will keep running forever

yeah earlier I assumed so but this fformat is provided by some other team in our company

You have redundant data in your json object.

which I cannot control unfortunately

yup and that is not under my control

some other team does that

Preprocess the json and remove the redundant data within your execute method. remove the first occuring "next".

do I need to do that? or can I not use this directly

          hasMore = jobject.get("next").getAsBoolean();

what I had in my code instead of your line?

My opinion?

yeah

IF the second next-key is only there IF the first next-value is TRUE. then get rid of redundant data.

what will be the benefit of doing this in execute method?

right now execute method just executes the url and return the response

now it needs to check next field and if it is false, then remove that field

otherwise keep that in json response

remove the first occuring next if it is a boolean. The benefit is to correct the data as early as possible so further processing has only ONE source of truth.

I see

The problem is that otherwise you pass the burden around until some has to handle it but does not expect it.

ok.. in this case, this will be the only class in whole my project that will be doing this and others will call it just to get the data back

It is the redundant data that makes the whole thing complicated. Think about that: You have to handle the JSON structure as an JsonArray. You cannot use a JsonObject because you have ambigous keys.

Using jobject.getAsJsonObject(KEY_NEXT).get(KEY_URL).getAsString() is luck.

lol

:-)

yeah you are right in terms of json structure but since the way it is, I cannot do much... I thought instead of worrying about all these details just make the code the way json it is so that it doesnt fail and return empty list if you cannot execute url for whatever reason. And that's what I did originally

but your suggestions are good

can you edit your answer accordingly for that next boolean as well just to make it complete?

You rely on the internal order to overwrite the key when you call:

jobject.getAsJsonObject(KEY_NEXT)

sure

yup

cu