Extracting superclass

[alexVengrovsk]

Signed-off-by: alextrentton@gmail.com

If-check in the getResourceBasedNode() method can be simplified. This change allows to exclude one check and protect method from NullPointerException. But this method still can throw this exception in line
Object o = selection.get(0);
I wasn't shure is it expected behaviour, so, didn't chenge anything there.

In the parseMessage() method it is posible to exclude the last if-check, because method indexOf() will return index or -1 if there is no searching sequence in the String.

One more question - both changed classes have identical methods with identical bodies: getResourceBasedNode(), isResourceAndStorableNode(), parseMessage(). Maybe, it would be better to create some utility class (for example - UploadUtil) with this methods and to use this class for both UploadFilePresenter and UploadFolderFromZipPresenter?
The same story about methods addFile() (from UploadFileViewImpl) and addFileUploadForm() (from UploadFolderFromZipViewImpl).

[codenvy-ci]

Can one of the admins verify this patch?

[skabashnyuk]

ok. @vparfonov @garagatyi ?

[alexVengrovsk]

Commit allows to exclude one more check - fileName.contains("\\") in the UploadFolderFromZipViewImpl and UploadFileViewImpl.

[garagatyi]

Thank you Alex for the contribution! Generally I'm OK with these fixes, but in my opinion these classes should be refactored instead of using util methods.
We've discussed that with @vparfonov. He is the lead of client-side in Che.
He will explain our thoughts soon.

[alexVengrovsk]

Ok. Yes, it is a better idea!

[vparfonov]

Hi thanks for you work. This is good catch form you side. And i totally agree with you that will be better create base class for Upload File views and move basic logic there.

[alexVengrovsk]

@vparfonov Do you want it as a superclass for them? If so, I can do this in this PR.

[vparfonov]

@alexVengrovsk superclass

[alexVengrovsk]

@vparfonov Ok. Will be done

[alexVengrovsk]

@vparfonov It is finished. Please, have a look.

[alexVengrovsk]

@vparfonov I resolved all conflicts. Can you check this PR?

[alexVengrovsk]

@skabashnyuk @garagatyi Sorry for bothering you, but is it something wrong with this PR? If so, tell me please and I will try to improve it.

[garagatyi]

As for me fact that some presenter extends some util doesn't look good. Utils should be used by composition, not inheritance. But it is up to @vparfonov because his expertise in client-side code is deeper.

[vparfonov]

yep will be better have something like BasicUploadPresenter or AbstractUploadPresenter instead of PresenterUtil

[alexVengrovsk]

@vparfonov It could be changed. Do you agree with @garagatyi about using composition instead of inheritance? Composition seems preferable for me too.

[alexVengrovsk]

@vparfonov I have renamed the PresenterUtil to the BasicUploadPresenter. I think, that now this PR is ready.

[vparfonov]

Not good use create presenter with constructor try please use injection instead. Thanks

[alexVengrovsk]

Ok

[alexVengrovsk]

@vparfonov I have changed instance creation with injection

[codenvy-ci]

Can one of the admins verify this patch?