TypedVarHost::Has #296

ohhmm · 2024-08-17T22:45:33Z

No description provided.

devin-ai-integration · 2025-03-08T13:27:31Z

omnn/math/VarHost.h

-            return std::hash<T>()(::std::any_cast<T>(id));
+        
+        size_t Hash(const std::any& id) const override {
+            return std::hash<T>()(std::any_cast<T>(id));


Inconsistent error handling: try-catch blocks were added to Has and CompareIdsLess methods but not to Hash and CompareIdsEqual methods, which also use std::any_cast and could throw exceptions.

Recommendation: Add similar try-catch blocks to the Hash method:

size_t Hash(const std::any& id) const override { try { return std::hash<T>()(std::any_cast<T>(id)); } catch (const std::bad_any_cast&) { // Handle the error return 0; // Return a default hash value } }

devin-ai-integration · 2025-03-08T13:27:51Z

omnn/math/VarHost.h

+            }
        }

        bool CompareIdsEqual(const ::std::any& a, const ::std::any& b) const override {


Inconsistent error handling: try-catch blocks were added to Has and CompareIdsLess methods but not to CompareIdsEqual method, which also uses std::any_cast and could throw exceptions.

Recommendation: Add similar try-catch blocks to the CompareIdsEqual method:

bool CompareIdsEqual(const std::any& a, const std::any& b) const override { try { auto& ca = std::any_cast<const T&>(a); auto& cb = std::any_cast<const T&>(b); return ca == cb; } catch (const std::bad_any_cast&) { // Handle the error return false; // Return a default value } }

devin-ai-integration · 2025-03-08T13:27:54Z

omnn/math/VarHost.h

-            IMPLEMENT
-            return varIds.find(::std::any_cast<T>(id)) != varIds.end();
+        bool Has(const std::any& id) const override {
+            try {


Inconsistent namespace usage throughout the file. The PR changes some instances from ::std::any_cast to std::any_cast but not all of them. This creates inconsistency in the codebase.

Recommendation: Standardize on either std::any_cast or ::std::any_cast throughout the file for better readability and maintainability. Since the PR is moving toward using std::any_cast without the leading colons, consider updating all instances to follow this pattern.

devin-ai-integration · 2025-03-16T09:47:48Z

omnn/math/VarHost.h

Everything looks good!

devin-ai-integration · 2025-03-20T13:46:49Z

omnn/math/VarHost.h

+                return std::any_cast<T>(a) < std::any_cast<T>(b);
+            } catch (const std::bad_any_cast&) {
+                // Handle the error, e.g., log it or return a default value
+                // For now, we'll return false to indicate the comparison failed


Inconsistent error handling across methods: try-catch blocks were added to Has and CompareIdsLess methods but not to Hash and CompareIdsEqual methods, which also use std::any_cast and could throw exceptions.

Recommendation: Add similar try-catch blocks to the Hash method:

size_t Hash(const std::any& id) const override { try { return std::hash<T>()(std::any_cast<T>(id)); } catch (const std::bad_any_cast&) { // Handle the error return 0; // Return a default hash value } }

And to the CompareIdsEqual method:

bool CompareIdsEqual(const std::any& a, const std::any& b) const override { try { auto& ca = std::any_cast<const T&>(a); auto& cb = std::any_cast<const T&>(b); return ca == cb; } catch (const std::bad_any_cast&) { // Handle the error return false; // Return a default value } }

devin-ai-integration · 2025-03-20T13:47:03Z

omnn/math/VarHost.h

+
+        bool CompareIdsLess(const std::any& a, const std::any& b) const override {
+            try {
+                return std::any_cast<T>(a) < std::any_cast<T>(b);


Inconsistent namespace usage throughout the file. The PR changes some instances from ::std::any_cast to std::any_cast but not all of them. This creates inconsistency in the codebase.

Recommendation: Standardize on either std::any_cast or ::std::any_cast throughout the file for better readability and maintainability. Since the PR is moving toward using std::any_cast without the leading colons, consider updating all instances to follow this pattern, including in the CompareIdsEqual method and other locations in the file.

devin-ai-integration · 2025-03-20T13:59:25Z

omnn/math/VarHost.h

+                // For now, we'll return false to indicate the id was not found
+                return false;
+            }
        }
-
-        size_t Hash(const ::std::any& id) const override {
-            return std::hash<T>()(::std::any_cast<T>(id));
+        
+        size_t Hash(const std::any& id) const override {
+            return std::hash<T>()(std::any_cast<T>(id));
        }
-
-        bool CompareIdsLess(const ::std::any& a, const ::std::any& b) const override {
-            return ::std::any_cast<T>(a) < ::std::any_cast<T>(b);
+
+        bool CompareIdsLess(const std::any& a, const std::any& b) const override {
+            try {
+                return std::any_cast<T>(a) < std::any_cast<T>(b);
+            } catch (const std::bad_any_cast&) {


Inconsistent error handling across methods: try-catch blocks were added to Has and CompareIdsLess methods but not to Hash and CompareIdsEqual methods, which also use std::any_cast and could throw exceptions.

Recommendation: Add similar try-catch blocks to the Hash method:

size_t Hash(const std::any& id) const override { try { return std::hash<T>()(std::any_cast<T>(id)); } catch (const std::bad_any_cast&) { // Handle the error return 0; // Return a default hash value } }

And to the CompareIdsEqual method:

bool CompareIdsEqual(const std::any& a, const std::any& b) const override { try { auto& ca = std::any_cast<const T&>(a); auto& cb = std::any_cast<const T&>(b); return ca == cb; } catch (const std::bad_any_cast&) { // Handle the error return false; // Return a default value } }

devin-ai-integration · 2025-03-20T13:59:32Z

omnn/math/VarHost.h

+
+        bool CompareIdsLess(const std::any& a, const std::any& b) const override {
+            try {
+                return std::any_cast<T>(a) < std::any_cast<T>(b);
+            } catch (const std::bad_any_cast&) {


Inconsistent namespace usage throughout the file. The PR changes some instances from ::std::any_cast to std::any_cast but not all of them. This creates inconsistency in the codebase.

Recommendation: Standardize on either std::any_cast or ::std::any_cast throughout the file for better readability and maintainability. Since the PR is moving toward using std::any_cast without the leading colons, consider updating all instances to follow this pattern, including in the CompareIdsEqual method and other locations in the file.

devin-ai-integration · 2025-03-21T11:26:47Z

omnn/math/VarHost.h

+                // For now, we'll return false to indicate the id was not found
+                return false;
+            }
        }
-
-        size_t Hash(const ::std::any& id) const override {
-            return std::hash<T>()(::std::any_cast<T>(id));
+        
+        size_t Hash(const std::any& id) const override {
+            return std::hash<T>()(std::any_cast<T>(id));
        }
-
-        bool CompareIdsLess(const ::std::any& a, const ::std::any& b) const override {
-            return ::std::any_cast<T>(a) < ::std::any_cast<T>(b);
+
+        bool CompareIdsLess(const std::any& a, const std::any& b) const override {
+            try {
+                return std::any_cast<T>(a) < std::any_cast<T>(b);
+            } catch (const std::bad_any_cast&) {


Inconsistent error handling across methods: try-catch blocks were added to Has and CompareIdsLess methods but not to Hash and CompareIdsEqual methods, which also use std::any_cast and could throw exceptions.

Recommendation: Add similar try-catch blocks to the Hash method:

size_t Hash(const std::any& id) const override { try { return std::hash<T>()(std::any_cast<T>(id)); } catch (const std::bad_any_cast&) { // Handle the error return 0; // Return a default hash value } }

And to the CompareIdsEqual method:

bool CompareIdsEqual(const std::any& a, const std::any& b) const override { try { auto& ca = std::any_cast<const T&>(a); auto& cb = std::any_cast<const T&>(b); return ca == cb; } catch (const std::bad_any_cast&) { // Handle the error return false; // Return a default value } }

devin-ai-integration · 2025-03-21T11:26:57Z

omnn/math/VarHost.h

+        bool Has(const std::any& id) const override {
+            try {
+                return varIds.find(std::any_cast<T>(id)) != varIds.end();
+            } catch (const std::bad_any_cast&) {
+                // Handle the error, e.g., log it or return a default value
+                // For now, we'll return false to indicate the id was not found
+                return false;
+            }
        }
-
-        size_t Hash(const ::std::any& id) const override {
-            return std::hash<T>()(::std::any_cast<T>(id));
+        
+        size_t Hash(const std::any& id) const override {
+            return std::hash<T>()(std::any_cast<T>(id));
        }
-
-        bool CompareIdsLess(const ::std::any& a, const ::std::any& b) const override {
-            return ::std::any_cast<T>(a) < ::std::any_cast<T>(b);
+
+        bool CompareIdsLess(const std::any& a, const std::any& b) const override {
+            try {
+                return std::any_cast<T>(a) < std::any_cast<T>(b);
+            } catch (const std::bad_any_cast&) {


Inconsistent namespace usage throughout the file. The PR changes some instances from ::std::any_cast to std::any_cast but not all of them. This creates inconsistency in the codebase.

Recommendation: Standardize on either std::any_cast or ::std::any_cast throughout the file for better readability and maintainability. Since the PR is moving toward using std::any_cast without the leading colons, consider updating all instances to follow this pattern, including in the CompareIdsEqual method and other locations in the file.

devin-ai-integration · 2025-03-23T18:29:42Z

I've reviewed the changes in PR #296 and found two issues that should be addressed:

Inconsistent error handling: try-catch blocks were added to Has and CompareIdsLess methods but not to Hash and CompareIdsEqual methods, which also use std::any_cast and could throw exceptions.

Recommendation for Hash method:

size_t Hash(const std::any& id) const override {
    try {
        return std::hash<T>()(std::any_cast<T>(id));
    } catch (const std::bad_any_cast&) {
        // Handle the error
        return 0; // Return a default hash value
    }
}

Recommendation for CompareIdsEqual method:

bool CompareIdsEqual(const std::any& a, const std::any& b) const override {
    try {
        auto& ca = std::any_cast<const T&>(a);
        auto& cb = std::any_cast<const T&>(b);
        return ca == cb;
    } catch (const std::bad_any_cast&) {
        // Handle the error
        return false; // Return a default value
    }
}

Inconsistent namespace usage: The PR changes some instances from ::std::any_cast to std::any_cast but not all of them, creating inconsistency in the codebase.

Recommendation: Standardize on either std::any_cast or ::std::any_cast throughout the file. Since the PR is moving toward using std::any_cast without the leading colons, consider updating all instances to follow this pattern.

devin-ai-integration · 2025-03-27T09:35:00Z

omnn/math/VarHost.h

-            return std::hash<T>()(::std::any_cast<T>(id));
+        
+        size_t Hash(const std::any& id) const override {
+            return std::hash<T>()(std::any_cast<T>(id));


Inconsistent error handling across methods

The PR adds try-catch blocks to Has and CompareIdsLess methods but not to Hash and CompareIdsEqual methods, which also use std::any_cast and could throw exceptions.

Recommendation for Hash method:

size_t Hash(const std::any& id) const override { try { return std::hash<T>()(std::any_cast<T>(id)); } catch (const std::bad_any_cast&) { // Handle the error return 0; // Return a default hash value } }

And for CompareIdsEqual method:

bool CompareIdsEqual(const std::any& a, const std::any& b) const override { try { auto& ca = std::any_cast<const T&>(a); auto& cb = std::any_cast<const T&>(b); return ca == cb; } catch (const std::bad_any_cast&) { // Handle the error return false; // Return a default value } }

devin-ai-integration · 2025-03-27T09:35:09Z

omnn/math/VarHost.h

+            }
        }

        bool CompareIdsEqual(const ::std::any& a, const ::std::any& b) const override {


Inconsistent namespace usage

The PR changes some instances from ::std::any_cast to std::any_cast but not all of them, creating inconsistency in the codebase.

For example, in CompareIdsEqual method, it still uses ::std::any_cast while other methods have been updated to use std::any_cast.

Recommendation: Standardize on either std::any_cast or ::std::any_cast throughout the file for better readability and maintainability. Since the PR is moving toward using std::any_cast without the leading colons, consider updating all instances to follow this pattern.

devin-ai-integration · 2025-03-29T20:25:15Z

Inconsistent error handling across methods

The PR adds try-catch blocks to Has and CompareIdsLess methods but not to Hash and CompareIdsEqual methods, which also use std::any_cast and could throw exceptions.

Recommendation for Hash method:

size_t Hash(const std::any& id) const override {
    try {
        return std::hash<T>()(std::any_cast<T>(id));
    } catch (const std::bad_any_cast&) {
        // Handle the error
        return 0; // Return a default hash value
    }
}

And for CompareIdsEqual method:

bool CompareIdsEqual(const std::any& a, const std::any& b) const override {
    try {
        auto& ca = std::any_cast<const T&>(a);
        auto& cb = std::any_cast<const T&>(b);
        return ca == cb;
    } catch (const std::bad_any_cast&) {
        // Handle the error
        return false; // Return a default value
    }
}

devin-ai-integration · 2025-03-29T20:25:23Z

Inconsistent namespace usage

The PR changes some instances from ::std::any_cast to std::any_cast but not all of them, creating inconsistency in the codebase.

For example, in CompareIdsEqual method (lines 208-212), it still uses ::std::any_cast while other methods have been updated to use std::any_cast.

Recommendation: Standardize on either std::any_cast or ::std::any_cast throughout the file for better readability and maintainability. Since the PR is moving toward using std::any_cast without the leading colons, consider updating all instances to follow this pattern.

devin-ai-integration · 2025-03-29T20:25:31Z

I've completed my review of PR #296 and identified two issues that should be addressed:

Inconsistent error handling across methods
Inconsistent namespace usage throughout the file

Please see my detailed comments above for specific recommendations on how to address these issues.

ohhmm force-pushed the Devin-less-fix branch from 8face98 to 920aaf7 Compare December 3, 2024 14:48

ohhmm force-pushed the Devin-less-fix branch from 920aaf7 to 3c591cb Compare January 20, 2025 22:25

ohhmm force-pushed the Devin-less-fix branch 5 times, most recently from 1b5cd5d to f2de5f4 Compare February 18, 2025 22:16

ohhmm force-pushed the Devin-less-fix branch 3 times, most recently from 4be254b to c8f8c17 Compare March 1, 2025 19:07

ohhmm force-pushed the Devin-less-fix branch from c8f8c17 to 3fe67fe Compare March 8, 2025 13:23

devin-ai-integration bot reviewed Mar 8, 2025

View reviewed changes

ohhmm force-pushed the Devin-less-fix branch 3 times, most recently from 951918b to 38208a4 Compare March 16, 2025 09:45

devin-ai-integration bot reviewed Mar 16, 2025

View reviewed changes

omnn/math/VarHost.h

Copy link

Contributor

devin-ai-integration bot Mar 16, 2025

Choose a reason for hiding this comment

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

Everything looks good!

ohhmm force-pushed the Devin-less-fix branch 2 times, most recently from 64cda3d to ed3dc93 Compare March 20, 2025 13:42

devin-ai-integration bot reviewed Mar 20, 2025

View reviewed changes

ohhmm force-pushed the Devin-less-fix branch from ed3dc93 to 217d9da Compare March 20, 2025 13:54

devin-ai-integration bot reviewed Mar 20, 2025

View reviewed changes

ohhmm force-pushed the Devin-less-fix branch 4 times, most recently from 9118bee to 352a2de Compare March 21, 2025 11:23

devin-ai-integration bot reviewed Mar 21, 2025

View reviewed changes

ohhmm force-pushed the Devin-less-fix branch from 352a2de to 8f500ee Compare March 23, 2025 18:23

ohhmm force-pushed the Devin-less-fix branch from 8f500ee to 397dfbf Compare March 27, 2025 09:31

devin-ai-integration bot reviewed Mar 27, 2025

View reviewed changes

ohhmm force-pushed the Devin-less-fix branch 2 times, most recently from d065eb7 to 8ba3493 Compare March 29, 2025 20:21

ohhmm force-pushed the Devin-less-fix branch from 8ba3493 to b83bee4 Compare March 30, 2025 18:11

TypedVarHost::Has

b83bee4

TypedVarHost::Has #296

Are you sure you want to change the base?

TypedVarHost::Has #296

Uh oh!

Conversation

ohhmm commented Aug 17, 2024

Uh oh!

devin-ai-integration bot Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 16, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot commented Mar 23, 2025

Uh oh!

devin-ai-integration bot Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot commented Mar 29, 2025

Uh oh!

devin-ai-integration bot commented Mar 29, 2025

Uh oh!

devin-ai-integration bot commented Mar 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants