Ensure that the important classes in Attack Graph, Language Graph, and Model modules have hash and repr methods. #118
Conversation
… sets of AttackGraphNodes
… graph, and language graph
maltoolbox/language/languagegraph.py
Outdated
|
|
||
| def __repr__(self) -> str: | ||
| return str(self.to_dict()) | ||
| return 'LanguageGraphAsset(name: "%s")' % self.name |
There was a problem hiding this comment.
It's a minor thing, but %-formatting is considered clunky, old and less preferred that f-strings and format. Maybe also for consistency it makes sense to stick to one formatting style e.g. f-strings and use format as a backup in cases where f-strings are not suitable?
There was a problem hiding this comment.
I will at one point lock all of you in a room and when only one sole survivor remains I will just take their advice. I don't remember whom it was that suggested that we switch over to the c-style strings in the debug logs.
There was a problem hiding this comment.
There is a confusing difference I only yesterday came to realize, that you do debug("string %s", var1) and not debug("string %s" % var1). The first does lazy loading (where if the debug function decides it needs to emit sth, it will then proceed with formatting the string), and the second is the same string formatting as if an f-string was used and it will have the same performance issues due to the string being formatted before being passed to the debug function, just for the function to decide not to use it if the log level is higher than debug.
There was a problem hiding this comment.
I think the issue is that you can't do the comma one for strings outside of the methods that know how to handle variable numbers of string parameters. It doesn't matter, I will just add this to the code cleanup issue and we will fix it when we get to it in 2077.
There was a problem hiding this comment.
We should still use c-style in log statements (for performance). For other cases we might use f-strings if it looks better since it does not affect performance either way.
There was a problem hiding this comment.
We should still use c-style in log statements (for performance). For other cases we might use f-strings if it looks better since it does not affect performance either way.
The important distinction is that arguments should not be given after a % but as arguments to the logging function.
There was a problem hiding this comment.
Yes, maybe c-style is the wrong term, not very used to c.
This PR removes the dataclass decorator from two important classes
AttackGraphNodeandAttacker.Another major change is that the
AttackGraphNodeclass now requires the id, it is no longer optional. Which in turn means thatAttackGraphNodesare now created as they are added to theAttackGraph, analogously to how theModelAssetsare created as they are added to aModel.