QuikGraph icon indicating copy to clipboard operation
QuikGraph copied to clipboard

Add TryGetX method to all algorithm implementations for external access to values stored in dictionaries

Open SimonTC opened this issue 4 years ago • 3 comments

As discussed in #13 all algorithm objects currently are leaking implementation details by making the dictionaries public properties. To be able to depreciate the direct access to the dictionaries in the future, new methods should be added to the algorithms for getting values in the dictionaries. In some algorithm objects there already is a TryGetColor for getting vertex colors. This approach should be used in all algorithms.

One thing to consider is if we also should have a TryGetXWithDefaultValue where a default value is returned if the vertex isn't found. It could cut down on boiler plate code for callers if they always want a value, but I'm not sure how important this is right now.

SimonTC avatar Aug 08 '20 12:08 SimonTC

Concerning the TryGetXWithDefaultValue we can also think about two methods in such situation meaning;

  • TryGetX(...) (would throw for example with vertex not found and no data associated)
  • TryGetX(..., ... defaultValue) (no throw, or maybe if the vertex is not part of the treated graph)

KeRNeLith avatar Aug 08 '20 13:08 KeRNeLith

When using the TryGetX naming I don't think we should throw exceptions since that would not be in line with how the functionality works with standard dictionaries. I do agree that setting the default value as part of the TryGetX makes sense.

Question is if we also should have a GetX method that does throw if the key doesn't exist? So we have both TryGetX that never will throw and TryX that does throw if the vertex doesn't exist / isn't accessible.

SimonTC avatar Aug 09 '20 09:08 SimonTC

Yeah you're right, that better fit the TryGet approach. In this case there are some implementation that may need to be updated. I was thinking about TryGetColor.

Considering the fact that TryGet does not throw having a Get version seems cool, like we have Parse and TryParse etc.

KeRNeLith avatar Aug 09 '20 15:08 KeRNeLith