Modify ↓

Opened 17 hours ago

Last modified 8 hours ago

#24951 new task

refactor: Fix Sonar issues in recent Grid Layer and OAuth

Reported by: DanProgs <Abenteurer.2901@…> Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: Cc:

Description

Attachments (2)

24951.patch​ (3.7 KB ) - added by DanProgs <Abenteurer.2901@…> 17 hours ago.
sonar-grid-oauth.patch​ (15.6 KB ) - added by wangi 8 hours ago.

Download all attachments as: .zip

Change History (7)

by DanProgs <Abenteurer.2901@…>, 17 hours ago

Attachment: 24951.patch​ added

comment:1 by wangi, 9 hours ago

Attached is a patch against r19646.

The deprecated URL constructor in OsmConnectionTest came from my patch for #24925. While I was looking at it I noticed that the other Sonar issues in the new code period also come from my grid patch (#8464, r19640), so this patch fixes those as well. It clears every issue in that list except the four S1309 ones about the existing PMD suppressions.

#24925 (this ticket)

  • OsmConnectionTest: new URL(…) → URI.create(…).toURL() (S1874)

#8464 grid

  • MapFrame.gridOverlay, ShowTileBordersAction.layer and GridPreference.TranslatedRenderer.name are now transient (S1948).
  • AlignGridRotationAction.getSelectedDirection() returns a shared EMPTY_DIRECTION constant instead of null (S1168). The callers and GridActionsTest now check the length. This is the same change as #24949, and the constant is taken from that patch, thanks DanProgs. This patch also updates the other two assertNull checks on getSelectedDirection() in GridActionsTest. The #24949 patch only changes the first, so the test would fail with it.
  • GridPreference: GBC.HORIZONTAL/GBC.BOTH → GridBagConstraints.HORIZONTAL/BOTH (S3252)
  • MapGridPaintable.getLatLonGridLines(): the meridian loop is moved out into addMeridian(), to go with the existing addParallel(). This brings the cognitive complexity down from 19 to under 15 (S3776).
  • MapGridPaintable.mapView: volatile → AtomicReference (S3077)
  • MapGridPaintable.projectionUnitsPerMetre(): removed the unnecessary (ILatLon) cast (S1905)
  • S1940 "use <=": the !(x > 0) checks were written that way on purpose so that NaN gets rejected too, and a plain x <= 0 would let NaN through. They now call a small isPositive() helper in MapGridPaintable. In GridPreference, where the value is parsed from user input, the check is Double.isNaN(value) || value <= 0. The one in addParallel() never sees NaN, so it is simply maxLon <= minLon.

There is no change in behaviour. GridActionsTest, MapGridPaintableTest, ShowTileBordersActionTest, TileSourceDisplaySettingsTest, GridPreferenceTest and OsmConnectionTest all pass, and checkstyle and ant pmd report nothing in the changed files.

Last edited 8 hours ago by wangi (previous) (diff)

comment:2 by wangi, 8 hours ago

Summary: refactor: replaces deprecated "URL" → refactor: Fix Sonar issues in recent Grid Layer and OAuth

comment:3 by DanProgs <Abenteurer.2901@…>, 8 hours ago

Please also take ticket #24949 into account in this context. It includes the change to new EastNorth[0] adapted by 'wangi', but implemented as a static constant named EMPTY_DIRECTION. Perhaps you could adjust this as well?

comment:4 by wangi, 8 hours ago

Ticket #24949 has been marked as a duplicate of this ticket.

by wangi, 8 hours ago

Attachment: sonar-grid-oauth.patch​ added

in reply to:  3 comment:5 by wangi, 8 hours ago

Replying to DanProgs <Abenteurer.2901@…>:

Please also take ticket #24949 into account in this context. It includes the change to new EastNorth[0] adapted by 'wangi', but implemented as a static constant named EMPTY_DIRECTION. Perhaps you could adjust this as well?

Patch and comment above updated. I've also closed off 24949 as duplicate now.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to DanProgs <Abenteurer.2901@…>.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.