Make getPropertyAsInt throw #3146

externl · 2024-11-13T16:15:34Z

This PR updates getPropertyAsInt to throw a PropertyException in the event that the property cannot be converted to an integer.

bernardnormier · 2024-11-13T20:24:58Z

cpp/include/Ice/Properties.h

@@ -82,15 +82,17 @@ namespace Ice
        std::string getPropertyWithDefault(std::string_view key, std::string_view value) noexcept;

        /**
-         * Get a property as an integer. If the property is not set, 0 is returned.
+         * Get a property as an integer. If the property is not set, 0 is returned. Throws PropertyException if


What the function throws belongs to the @throws tag, not the summary.

bernardnormier · 2024-11-13T20:25:07Z

cpp/include/Ice/Properties.h


        /**
-         * Get an Ice property as an integer. If the property is not set, its default value is returned.
+         * Get an Ice property as an integer. If the property is not set, its default value is returned. Throws


bernardnormier · 2024-11-13T20:26:24Z

cpp/include/Ice/Properties.h

@@ -99,13 +101,14 @@ namespace Ice
        int getIcePropertyAsInt(std::string_view key);

        /**
-         * Get a property as an integer. If the property is not set, the given default value is returned.
+         * Get a property as an integer. If the property is not set, the given default value is returned. Throws
+         * PropertyException if the property value is not a valid integer.


bernardnormier · 2024-11-13T20:27:43Z

csharp/src/Ice/Properties.cs

@@ -165,15 +165,17 @@ public string getPropertyWithDefault(string key, string value)

    /// <summary>
    /// Get a property as an integer.
-    /// If the property is not set, 0 is returned.
+    /// If the property is not set, 0 is returned. Throws PropertyException if the property


The summary should not describe what the method throws.

Still have to move the exception docs to exception tag

bernardnormier · 2024-11-13T20:29:06Z

java/src/com.zeroc.ice/src/main/java/com/zeroc/Ice/Properties.java

@@ -215,6 +216,7 @@ public int getPropertyAsInt(String key) {
     *
     * @param key The property key.
     * @return The property value interpreted as an integer, or the default value.
+     * @throws PropertyException Raised if the property value is not a valid integer.


Would be good to document what we throw if the key doesn't designate a valid Ice property.

bernardnormier · 2024-11-13T20:30:56Z

php/test/Ice/properties/Client.php

@@ -70,5 +70,15 @@ function run($args)
            test($ex->getMessage() == "unknown Ice property: Ice.UnknownProperty");
        }
        echo "ok\n";
+
+        echo "testing that trying to read a non-numeric value as an int throws... ";
+        try {


The try block should not include the "arrange" part of the test.

bernardnormier · 2024-11-13T20:31:38Z

ruby/test/Ice/properties/Client.rb

@@ -101,5 +101,15 @@ def run(args)
        end
        puts "ok"

+        print "testing that trying to read a non-numeric value as an int throws... "
+        begin
+            properties = Ice.createProperties(args)


bernardnormier · 2024-11-13T20:34:01Z

swift/src/Ice/Properties.swift

@@ -34,14 +34,14 @@ public protocol Properties: AnyObject {
    /// - parameter _: `String` The property key.
    ///
    /// - returns: `Int32` - The property value interpreted as an integer.


Need to document exception.

bernardnormier · 2024-11-13T20:34:11Z

swift/src/Ice/Properties.swift


    /// Get an Ice property as an integer. If the property is not set,its default value is returned.
    ///
    /// - parameter key: `String` The property key.
    ///
    /// - returns: `Int32` - The property value interpreted as an integer, or the default value.
-    func getIcePropertyAsInt(_ key: String) -> Int32
+    func getIcePropertyAsInt(_ key: String) throws -> Int32


pepone

Seems you still have to update JavaScript

pepone · 2024-11-16T10:02:39Z

csharp/src/Ice/Properties.cs

@@ -165,15 +165,17 @@ public string getPropertyWithDefault(string key, string value)

    /// <summary>
    /// Get a property as an integer.
-    /// If the property is not set, 0 is returned.
+    /// If the property is not set, 0 is returned. Throws PropertyException if the property


Still have to move the exception docs to exception tag

pepone · 2024-11-16T10:03:06Z

csharp/src/Ice/Properties.cs

    /// </summary>
    /// <param name="key">The property key.</param>
    /// <returns>The property value interpreted as an integer.</returns>
    public int getPropertyAsInt(string key) => getPropertyAsIntWithDefault(key, 0);

    /// <summary>
    /// Get an Ice property as an integer.
-    /// If the property is not set, its default value is returned.
+    /// If the property is not set, its default value is returned. Throws PropertyException if the property is not a
+    /// known Ice property or the value is not a valid integer.


ditto, move the exception docs to exception tag

…e/ice#3146)

externl added 3 commits November 13, 2024 11:15

Make getPropertyAsInt throw

45f58dc

Formatting

f059d8a

Merge branch 'main' into make-getPropertyAsInt-throw

c344270

externl marked this pull request as ready for review November 13, 2024 18:58

bernardnormier self-requested a review November 13, 2024 20:23

externl requested a review from pepone November 13, 2024 20:26

bernardnormier approved these changes Nov 13, 2024

View reviewed changes

pepone approved these changes Nov 13, 2024

View reviewed changes

externl added 2 commits November 15, 2024 09:17

Fixes

418ea56

JS fixes

3135536

externl requested a review from pepone November 15, 2024 14:54

prettier

5a2b635

pepone approved these changes Nov 16, 2024

View reviewed changes

externl added 2 commits November 18, 2024 09:01

review fix

b17d65f

dotent format

1fcf4ba

externl merged commit 892f92b into zeroc-ice:main Nov 18, 2024
19 checks passed

externl deleted the make-getPropertyAsInt-throw branch November 18, 2024 15:25

InsertCreativityHere pushed a commit to InsertCreativityHere/compiler-comparison that referenced this pull request Dec 26, 2024

HEAD:(zeroc-ice/ice@892f92bab0) Make getPropertyAsInt throw (zeroc-ic…

ddbcf4c

…e/ice#3146)

InsertCreativityHere pushed a commit to InsertCreativityHere/compiler-comparison that referenced this pull request Jan 1, 2025

HEAD:(zeroc-ice/ice@892f92bab0) Make getPropertyAsInt throw (zeroc-ic…

e6ca9a0

…e/ice#3146)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make getPropertyAsInt throw #3146

Make getPropertyAsInt throw #3146

externl commented Nov 13, 2024 •

edited

Loading

bernardnormier Nov 13, 2024

bernardnormier Nov 13, 2024

bernardnormier Nov 13, 2024

bernardnormier Nov 13, 2024

pepone Nov 16, 2024

bernardnormier Nov 13, 2024

bernardnormier Nov 13, 2024

bernardnormier Nov 13, 2024

bernardnormier Nov 13, 2024

bernardnormier Nov 13, 2024

pepone left a comment

pepone Nov 16, 2024

pepone Nov 16, 2024

Make getPropertyAsInt throw #3146

Make getPropertyAsInt throw #3146

Conversation

externl commented Nov 13, 2024 • edited Loading

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

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

pepone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

externl commented Nov 13, 2024 •

edited

Loading