-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enable library user to construct network programmatically #12
Comments
Well, this is advanced IT stuff for me -- I'm still at a CS 101 level. But just to clarify, the use of the INP file format is embedded in the API function There is an additional factor that bears mentioning. Right now, the API is designed using a least-common-denominator C-style interface. This means that it can't pass objects as arguments or include overloaded functions. The reasoning behind this choice is the subject for a separate discussion. The end result is that it puts some limitations on what the API library can accomplish. |
Yes, what is currently implemented is pretty close to option (2) - though to implement this enhancement, I'd want to peel it away from the main library and put it in a supporting (separate) library that gets linked in as needed. The C API is a good idea, and might even be a way to have a version-2 backwards-compatible library. For fully C++ clients, though, we should think about the public class interfaces and whether they offer a good set of methods to support our use-cases. |
My C++ knowledge is mediocre at best, but I would fully support having this and would try to helps as much as I can. I come from the Python world, so having things like (oversimplified example): from pyepanet import Network ,Config
conf = Config()
n = Network(name='My awesome network', config=conf)
n1 = n.add_tank(x=0, y=0, name='some name')
n2 = n.add_node(x=1, y=1, name='some other name')
p1 = n.add_pipe(n1, n2, diameter=0.2)
# or stuff like
n.add_nodes([('name', x=5, y=6), ('name1', x=10, y=8)]
for p in n.pipes():
print(p)
for no in n.nodes():
print(no) Is expected, is desired and is pythonic :-p. I actually started something like this for fun using the C toolkit, but at the end all this work was translated to a input... which is/was very limited. I would advice taking a look at Qt framework, their API is very well done with the right amount of what should be part of the constructor and what should be part of methods (setters and getters) This is why OOPNET work might be very useful if disclosed, as it could already provide some background on how to structure this (and moving it back to C++ ?) |
This is a needed feature of the library that would make it appealing for many uses - commercial and research. Rather than depending on the
inp
format, client code should be allowed to construct a network/project using different mechanisms. Examples include a geo-database / sqlite file, json data, a web api, protocol buffer, a network stored in RAM, or some proprietary format. Ideally for performance reasons, the main library code would not be hitting the disk for I/O.Currently, it looks like network construction is spread across
Project::load
and theXxParser
classes, and theinp
file dependency is baked-in to the Project implementation. While much of the existing network-building functionality could be duplicated for different sorts of deserialization schemes, repetition could be minimized in a few different ways:inp
datasource adapter should be included.There are benefits to both approaches. Approach (1) maintains library control of how a network is constructed and ensures that the client provides the needed data (or else an error occurs). It is technically more OO because the user just has to create a concrete class derived from the interface that implements the appropriate logic, but approach (2) is probably more flexible and would force the library to be robust enough to handle the multitude of use cases out there; it is more procedural and gives more responsibility and power to the user.
Any thoughts on this? It's an important piece to get right.
The text was updated successfully, but these errors were encountered: