builtin: Add `is_hex()`, `is_int()`, `is_bin()`, and `is_oct()` by strings - Feature #20540

viniciusfdasilva · 2024-01-15T02:05:31Z

Code

// Check if a string is an octal value. Returns 'true' if it is, or 'false' if it is not
@[direct_array_access]
pub fn (str string) is_oct() bool {
	mut i := 0

	if str[i] == `0` {
		i++
	} else if str[i] == `-` || str[i] == `+` {
		i++

		if str[i] == `0` {
			i++
		} else {
			return false
		}
	} else {
		return false
	}

	if str[i] == `o` {
		i++
	} else {
		return false
	}

	if i == str.len {
		return false
	}

	for i < str.len {
		if str[i] < `0` || str[i] > `7` {
			return false
		}
		i++
	}

	return true
}

// Check if a string is an binary value. Returns 'true' if it is, or 'false' if it is not
@[direct_array_access]
pub fn (str string) is_bin() bool {
	mut i := 0

	if str[i] == `0` {
		i++
	} else if str[i] == `-` || str[i] == `+` {
		i++

		if str[i] == `0` {
			i++
		} else {
			return false
		}
	} else {
		return false
	}

	if str[i] == `b` {
		i++
	} else {
		return false
	}

	if i == str.len {
		return false
	}

	for i < str.len {
		if str[i] < `0` || str[i] > `1` {
			return false
		}
		i++
	}

	return true
}

// Check if a string is an hexadecimal value. Returns 'true' if it is, or 'false' if it is not
@[direct_array_access]
pub fn (str string) is_hex() bool {
	mut i := 0

	if str[i] == `0` {
		i++
	} else if str[i] == `-` || str[i] == `+` {
		i++

		if str[i] == `0` {
			i++
		} else {
			return false
		}
	} else {
		return false
	}

	if str[i] == `x` {
		i++
	} else {
		return false
	}

	if i == str.len {
		return false
	}

	for i < str.len {
		if (str[i] < `0` || str[i] > `9`) && ((str[i] < `a` || str[i] > `f`)
			&& (str[i] < `A` || str[i] > `F`)) {
			return false
		}
		i++
	}

	return true
}

// Check if a string is an integer value. Returns 'true' if it is, or 'false' if it is not
@[direct_array_access]
pub fn (str string) is_int() bool {
	mut i := 0

	if (str[i] != `-` && str[i] != `+`) && (!str[i].is_digit()) {
		return false
	} else {
		i++
	}

	if i == str.len && (!str[i - 1].is_digit()) {
		return false
	}

	for i < str.len {
		if str[i] < `0` || str[i] > `9` {
			return false
		}
		i++
	}

	return true
}

Test code

// vlib/builtin/string_test.v file
fn test_is_bin(){

	assert '0b1'.is_bin()       == true
	assert '0b0'.is_bin()       == true
	assert '0b'.is_bin()        == false
	assert '-0b'.is_bin()       == false
	assert '-0b1'.is_bin()      == true
	assert '-0b0'.is_bin()      == true
	assert '-0b1101'.is_bin()   == true
	assert '-0b0101'.is_bin()   == true
	assert '-324'.is_bin()      == false
	assert '-0'.is_bin()        == false
	assert '0x1'.is_bin()       == false
	assert '0x1A'.is_bin()      == false
	assert '-0x1'.is_bin()      == false
	assert '-0x1A'.is_bin()     == false
	assert '0x'.is_bin()        == false
	assert '0'.is_bin()         == false
	assert '0xFF'.is_bin()      == false
	assert '0xG'.is_bin()       == false
	assert '-0xFF'.is_bin()     == false
	assert '0.34'.is_bin()      == false
	assert '0o23'.is_bin()      == false
	assert 'vlang'.is_bin()     == false
}

fn test_is_oct(){

	assert '0o0'.is_oct()       == true
	assert '0o1'.is_oct()       == true
	assert '0o2'.is_oct()       == true
	assert '-0o0'.is_oct()      == true
	assert '-0o1'.is_oct()      == true
	assert '-0o2'.is_oct()      == true

	assert '0o04'.is_oct()      == true
	assert '0o16'.is_oct()      == true
	assert '0o23'.is_oct()      == true
	assert '-0o05'.is_oct()     == true
	assert '-0o13'.is_oct()     == true
	assert '-0o22'.is_oct()     == true

	assert '0o8'.is_oct()       == false
	assert '0o9'.is_oct()       == false
	assert '-0o8'.is_oct()      == false
	assert '-0o9'.is_oct()      == false
	assert '0o84'.is_oct()      == false
	assert '0o96'.is_oct()      == false
	assert '-0o83'.is_oct()     == false
	assert '-0o2923'.is_oct()   == false
	assert '0b1'.is_oct()       == false
	assert '0b0'.is_oct()       == false
	assert '0b'.is_oct()        == false
	assert '-0b'.is_oct()       == false
	assert '-0b1'.is_oct()      == false
	assert '-0b0'.is_oct()      == false
	assert '-0b1101'.is_oct()   == false
	assert '-0b0101'.is_oct()   == false
	assert '-324'.is_oct()      == false
	assert '-0'.is_oct()        == false
	assert '0x1'.is_oct()       == false
	assert '0x1A'.is_oct()      == false
	assert '-0x1'.is_oct()      == false
	assert '-0x1A'.is_oct()     == false
	assert '0x'.is_oct()        == false
	assert '0'.is_oct()         == false
	assert '0xFF'.is_oct()      == false
	assert '0xG'.is_oct()       == false
	assert '-0xFF'.is_oct()     == false
	assert '0.34'.is_oct()      == false
	assert 'vlang'.is_oct()     == false
}

fn test_is_hex(){

	assert '-324'.is_hex()       == false
	assert '-0'.is_hex()         == false
	assert '0x1'.is_hex()        == true
	assert '0x1A'.is_hex()       == true
	assert '-0x1'.is_hex()       == true
	assert '-0x1A'.is_hex()      == true
	assert '0x'.is_hex()         == false
	assert '0'.is_hex()          == false
	assert '0xFF'.is_hex()       == true
	assert '0xG'.is_hex()        == false
	assert '-0xFF'.is_hex()      == true
	assert '0b1101'.is_hex()     == false
	assert '0.34'.is_hex()       == false
	assert '0o23'.is_hex()       == false
	assert 'vlang'.is_hex()      == false
}

fn test_is_int(){

	assert '-324'.is_int()       == true
	assert '234'.is_int()        == true
	assert '-0'.is_int()         == true
	assert '-b'.is_int()         == false
	assert '-123456789'.is_int() == true
	assert '123456789'.is_int()  == true
	assert '0x1'.is_int()        == false
	assert '0b1101'.is_int()     == false
	assert '0.34'.is_int()       == false
	assert '0o23'.is_int()       == false
	assert 'vlang'.is_int()      == false
}

Test output

…rings on builtin module

JalonSolov · 2024-01-15T02:10:32Z

You'll need to format your changes.

viniciusfdasilva · 2024-01-15T02:20:32Z

@JalonSolov Any suggestions on what I should do?

JalonSolov · 2024-01-15T02:28:23Z

Just run v fmt -w . in your playpen, and commit the reformatted files.

JalonSolov · 2024-01-15T02:32:05Z

Note that you can install a git hook to check the files when you try to commit them by running cp <v install dir>/cmd/tools/git_pre_commit_hook.vsh <your clone>/.git/hooks/pre-commit

viniciusfdasilva · 2024-01-15T02:38:02Z

@JalonSolov v fmt -w . executed!

JalonSolov · 2024-01-15T04:09:30Z

Still says string_test.v is not formatted.

viniciusfdasilva · 2024-01-15T10:09:21Z

@JalonSolov Sorry! I had forgotten!

penguindark · 2024-01-15T12:30:43Z

my only concern as always is performance,
anyway good job. 👍

viniciusfdasilva · 2024-01-15T12:47:58Z

@penguindark

"I do too! In this case, the implementation meets this requirement, in my view, by eliminating any other invalid formats in the checks before the for loop! =)

If I find a better way, I will submit a new pull request with the improvement! 😀

penguindark

After a quick check on the code I'm suggesting to rewrite the functions in a more efficent way.
At my first sight I missed the string allocation in the function.
Allocate in the Heap for a check on a string it is truly not recommended IMHO.
Anyway I appreciate a lot the effort the @viniciusfdasilva put in this code 👍
Thanks a lot for your effort to contribute to V.

vlib/builtin/string.v

penguindark · 2024-01-15T18:36:22Z

vlib/builtin/string_test.v

@@ -425,6 +425,109 @@ fn test_rsplit_once() ? {
 	assert ext3 == ''
 }

+fn test_is_bin() {


Think if the cases like +0b1001 or +0xFA3 must be managed

Yes! I find the problem! kkkkk

viniciusfdasilva · 2024-01-15T19:14:41Z

@penguindark

I appreciate the recognition for the effort in my collaboration! I'm just starting with the language! I've only had 1 month of exposure to it, and I'm happy to contribute to the language's development! :)

…ggestions

penguindark

For me it is ok!
Tnx for your work :)

JalonSolov · 2024-01-16T04:29:45Z

vlib/builtin/string.v

+	for i < str.len {
+		if str[i] < `0` || str[i] > `7` {
+			return false
+		}
+		i++
+	}


More idiomatic (and slightly shorter) V would be

Suggested change

for i < str.len {

if str[i] < `0` || str[i] > `7` {

return false

}

i++

}

for c in str[i..] {

if c < `0` || c > `7` {

return false

}

}

less performant I suppose due the str[i..] -_-
We can increase speed further using pointers.

@JalonSolov

for c in str[i..]

I agree with @penguindark. I think direct index access might be more performant by avoiding the cost of generating the slice in my opinion! I suggest sticking to the original implementation.

If this were the only change, true. However, you could move the entire function inside for c in str and avoid using the index at all.

However, that can be done later.

@JalonSolov and @penguindark

Of course! Thank you for the suggestion! I request approval for this PR! I plan to work on some other similar functions as well!

Let's analyze later which aspects can lead one approach to be better than another! :)

I appreciate both of you for the suggestions! 👍

the problem of the str[i..] is that it generate a copy of the data,
at the actual compiler state it is very inefficient.

I think we can proceed with the merge of this PR.

penguindark

For me it is mergiable

spytheman · 2024-01-16T21:46:49Z

vlib/builtin/string.v

+// Check if a string is an octal value. Returns 'true' if it is, or 'false' if it is not
+@[direct_array_access]
+pub fn (str string) is_oct() bool {
+	mut i := 0


Suggested change

mut i := 0

if str.len == 0 {

return false

}

mut i := 0

Otherwise str[i] on the next line will read the 0th byte that V requires for all V strings, for compatibility with C, which is ok, but not ideal (there are some situations where V strings may be generated inside unsafe{} blocks, for internal use, that will not have that 0th byte at the end, and it will be hard to track which string methods need modification for that case, and which do not).

spytheman · 2024-01-16T21:53:22Z

vlib/builtin/string.v

+// Check if a string is an binary value. Returns 'true' if it is, or 'false' if it is not
+@[direct_array_access]
+pub fn (str string) is_bin() bool {
+	mut i := 0


Same, if str.len == 0 { return false } should be here as well.

spytheman · 2024-01-16T21:54:23Z

vlib/builtin/string.v

+// Check if a string is an hexadecimal value. Returns 'true' if it is, or 'false' if it is not
+@[direct_array_access]
+pub fn (str string) is_hex() bool {
+	mut i := 0


Add if str.len == 0 { return false } at the start.

spytheman · 2024-01-16T21:54:38Z

vlib/builtin/string.v

+// Check if a string is an integer value. Returns 'true' if it is, or 'false' if it is not
+@[direct_array_access]
+pub fn (str string) is_int() bool {
+	mut i := 0


Add if str.len == 0 { return false } at the start.

spytheman · 2024-01-16T21:55:38Z

vlib/builtin/string_test.v

@@ -425,6 +425,168 @@ fn test_rsplit_once() ? {
 	assert ext3 == ''
 }

+fn test_is_bin() {
+	assert '0b1'.is_bin() == true


Please also assert on the ''.is_bin() etc cases too.

spytheman

Good work.

…tc methods, suggested on PR #20540 (#20564)

Vinicius Silva and others added 2 commits January 14, 2024 22:49

Feature - is_bin(), is_hex(), is_oct() and is_int() implemented to st…

ebea29e

…rings on builtin module

Replace string range verification by string.starts_with()

d322f0c

'v fmt -w' command executed

45a9550

Reformatted file string_test.v

8bb0b72

penguindark requested changes Jan 15, 2024

View reviewed changes

Fix value with '+' return false and implemented PR #20540 reviewer su…

c7319dd

…ggestions

viniciusfdasilva requested a review from penguindark January 15, 2024 23:42

penguindark approved these changes Jan 16, 2024

View reviewed changes

JalonSolov reviewed Jan 16, 2024

View reviewed changes

viniciusfdasilva requested review from penguindark and JalonSolov January 16, 2024 12:50

JalonSolov requested a review from spytheman January 16, 2024 14:52

penguindark approved these changes Jan 16, 2024

View reviewed changes

spytheman reviewed Jan 16, 2024

View reviewed changes

spytheman approved these changes Jan 16, 2024

View reviewed changes

spytheman merged commit afd74ad into vlang:master Jan 16, 2024
54 checks passed

viniciusfdasilva mentioned this pull request Jan 17, 2024

builtin: Add empty string verification suggested on PR #20540 #20564

Merged

spytheman pushed a commit that referenced this pull request Jan 17, 2024

builtin: add empty string verification for the new string .is_oct() e…

3569635

…tc methods, suggested on PR #20540 (#20564)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

builtin: Add `is_hex()`, `is_int()`, `is_bin()`, and `is_oct()` by strings - Feature #20540

builtin: Add `is_hex()`, `is_int()`, `is_bin()`, and `is_oct()` by strings - Feature #20540

viniciusfdasilva commented Jan 15, 2024 •

edited

JalonSolov commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024

JalonSolov commented Jan 15, 2024

JalonSolov commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024

JalonSolov commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024

penguindark commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024 •

edited

penguindark left a comment •

edited

penguindark Jan 15, 2024

viniciusfdasilva Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024

penguindark left a comment

JalonSolov Jan 16, 2024

penguindark Jan 16, 2024 •

edited

viniciusfdasilva Jan 16, 2024

JalonSolov Jan 16, 2024

viniciusfdasilva Jan 16, 2024 •

edited

penguindark Jan 16, 2024

penguindark left a comment

spytheman Jan 16, 2024

spytheman Jan 16, 2024 •

edited

spytheman Jan 16, 2024

spytheman Jan 16, 2024

spytheman Jan 16, 2024

spytheman Jan 16, 2024

spytheman left a comment

builtin: Add is_hex(), is_int(), is_bin(), and is_oct() by strings - Feature #20540

builtin: Add is_hex(), is_int(), is_bin(), and is_oct() by strings - Feature #20540

Conversation

viniciusfdasilva commented Jan 15, 2024 • edited

Code

Test code

Test output

JalonSolov commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024

JalonSolov commented Jan 15, 2024

JalonSolov commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024

JalonSolov commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024

penguindark commented Jan 15, 2024

viniciusfdasilva commented Jan 15, 2024 • edited

penguindark left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viniciusfdasilva commented Jan 15, 2024

penguindark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penguindark Jan 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viniciusfdasilva Jan 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penguindark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spytheman Jan 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spytheman left a comment

Choose a reason for hiding this comment

builtin: Add `is_hex()`, `is_int()`, `is_bin()`, and `is_oct()` by strings - Feature #20540

builtin: Add `is_hex()`, `is_int()`, `is_bin()`, and `is_oct()` by strings - Feature #20540

viniciusfdasilva commented Jan 15, 2024 •

edited

viniciusfdasilva commented Jan 15, 2024 •

edited

penguindark left a comment •

edited

penguindark Jan 16, 2024 •

edited

viniciusfdasilva Jan 16, 2024 •

edited

spytheman Jan 16, 2024 •

edited