Use SWT constant and make the code more readable.

[deepika-u]

This commit refactors the classes in swt by using swt provided constant and makes the code more readable. Instead, the SWT.SPACE static constant is now leveraged for representing a string containing single character which is space in these classes.

[github-actions]

Test Results

   494 files  ±0     494 suites  ±0   8m 48s ⏱️ -50s
 4 333 tests ±0   4 320 ✅ ±0   13 💤 ±0  0 ❌ ±0 
16 574 runs  ±0  16 466 ✅ ±0  108 💤 ±0  0 ❌ ±0 

Results for commit a57288a. ± Comparison against base commit 4a201e2.

♻️ This comment has been updated with latest results.

[deepika-u]

@merks @HeikoKlare : can you review this please?

[HeikoKlare]

I am not sure about this change, as for me several questions arise:

• Is there a specific reason for doing this with SWT.SPACE and not every other character constant in SWT, like SWT.TAB and the like?
• Why replace exactly these usages of ' ' and not the others within the SWT bundles?
• Why is that reasonable/necessary now? Is there a specific trigger or did you come across this by accident? I am asking because the constant has been there for more than 10 years now.

To me, it looks as if those SWT constants are rather used for expressing key codes and not as constants for general ASCII characters. If we really want to use them for that, I would expect that we do that for every appearance of ' ' or that we have a common notion of where we use it and where we don't.

[merks]

I don't think this makes the code more readable. It makes the code more verbose and hence even arguably less readable.

[deepika-u]

To me, it looks as if those SWT constants are rather used for expressing key codes and not as constants for general ASCII characters. If we really want to use them for that, I would expect that we do that for every appearance of ' ' or that we have a common notion of where we use it and where we don't.

Actually i totally agree to your view point. While checking on another issue, i got to check on the contents of the SWT.java file indetail so landed into this pr actually. While updating the instances of SPACE occurences only i got see '\t', '\n' characters are also being used. By the way i didnt find any contant for an empty string like ZERO_LENGTH_STRING or EMPTY_STRING, do you think we should add one?

• At first place, i have did only for windows(x86_64) platform for which i missed 1 instance, i have added it now. But now i updated in other OS specific files as well wherever needed.
• I was not sure on the impact(in terms of performance) of replacing space character by this constant in os calls(Text.java, Menu.java, Combo.java kind of files) so i have left them untouched.
• Ofcourse i didnt update in commented(Text.java) sections of code.
• When individual characters are mentioned like occurrences in JSON.java, WebBrowser.java - i havent intentionally changed those so that it is more sensible that way itself.

Ofcourse this is a cosmetic change and nothing else should change except the readability for a developer post this pr.

[deepika-u]

I don't think this makes the code more readable. It makes the code more verbose and hence even arguably less readable.

I am open to take this pr in any direction, thats individual perception in my opinion. But thanks for your opinion on this.

[laeubi]

I don't think this is appropriate here.

If I look at the javadoc it says:

ASCII character convenience constant for the space character

and comparing this with other like ESC and how it is used in the current codebase it becomes quite clear that this is for using in key events when the user has pressed the space-bar an not a general purpose constant to be used in programming code.

Taking this further, one soon need to replace the : or ; as well and literally each and every single character. I also don't think it improves readability, if I see a space I can easily read it as a space (ignoring the fact that there are different space characters like non breakable space and alike for now), also there is no risk that a space changes to something else and we want it to be the case very where (like with a string constant).

If one really want to improve readability, I see some places where a loop is used and instead one maybe want to use " ".repeat(n) instead.