Skip to content

Fix code-viewer __compare - #50

Merged
mlopezFC merged 5 commits into
masterfrom
fix-compare-version
Jul 19, 2023
Merged

Fix code-viewer __compare#50
mlopezFC merged 5 commits into
masterfrom
fix-compare-version

Conversation

@javier-godoy

@javier-godoyjavier-godoy commented Jul 19, 2023

Copy link
Copy Markdown
Member

The current implementation of version comparison in #44 follows a lexicographic order, yielding consistent yet non-intuitive outcomes (23.0.0 is greater than 23 and 23.0.0 is not equal to 23)

__compare(a: string,b:string) : number{
letaa=a.split('.');
letbb=b.split('.');
for(leti=0;i<Math.min(aa.length,bb.length);i++){
letai=parseInt(aa[i]);
letbi=parseInt(bb[i]);
if(ai<bi)return-1;
if(ai>bi)return+1;
}
if(aa.length<bb.length)return-1;
if(aa.length>bb.length)return+1;
return0;

Instead, we should only compare the provided digits, so that 23.0.0 is not greater than 23, and 23.1.0 is equals to 23.

- if (aa.length<bb.length) return -1; - if (aa.length>bb.length) return +1; 
return 0; 

Strictly, "equals" will be implemented as "is contained in" (23.0.0 is-contained-in 23) and "greater-or-equal" will be implemented as "precedes or is contained in" ("23.4.5 precedes 24", 23.4.5 "precedes or is contained in 23"), thus "equality" will not be transitive, but I think we can keep the usual operator names because they are intuitive.

@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed! Quality Gate passed

BugA0 Bugs
VulnerabilityA0 Vulnerabilities
Security HotspotA0 Security Hotspots
Code SmellA0 Code Smells

No Coverage information No Coverage information
0.0%0.0% Duplication

@mlopezFCmlopezFC left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mlopezFC
mlopezFC merged commit e380edd into masterJul 19, 2023
@mlopezFC
mlopezFC deleted the fix-compare-version branch July 19, 2023 19:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@javier-godoy@mlopezFC