search

fuzzylite / jfuzzylite

jfuzzylite: a fuzzy logic control library in Java
https://fuzzylite.com/
GNU General Public License v3.0
42 stars 21 forks source link

Support import from inputstream #12

Closed ssaarinen closed 6 years ago

ssaarinen commented 6 years ago

This is more flexible than importing from files only. Thought about adding also import from BufferedReader so caller could decide to change encoding but left it out for now.

jcrada commented 6 years ago

Thanks for this contribution. I am still thinking on it. I like it, but I also like to have a standard across libraries in C++ and Java. Could we perhaps add a method to the Op.java to convert the resource to a sort of file instead and pass it as arguments? What are your thoughts?

ssaarinen commented 6 years ago

I think writing a resource to a file just to get it to import as a file is totally backwards. As you can see from the implementation importing from stream is more general than importing from file. You can try to create a version where you convert a stream to a file and import from there and see how 'wrong' it looks :). Further more you might even not have a filesystem in your env where you need to run this (AWS lambda for example)

File represents a file in a filesystem but when it comes to importing a rule engine we are not actually interested in the file but the bits inside a file that are in Java (and to my knowledge on C++ 10+ years back) represented as a stream.

jcrada commented 6 years ago

Hey, I did not mean to write the resource file to a file. I meant to treat the file in the resource as the file it actually is. Alternatively, I even prefer to have a method in Op that gets the text of a resource file and use the fromString method before adding the stream to this class. For me, it is just about simplicity and I cannot see the need to further extend this rather simple class.

ssaarinen commented 6 years ago

Sorry I misinterpreted your writing on the resource case. But I still think that import/export from/to stream would be good to have. You can take a look foor eg. json or xml parsing libraries that all provide means to do this. My initial reaction to this was that it seems weird that importer has import from file but not from inputstream.

I can do inpustream->string with guava or commons-io or million other ways so I think adding it to Op would be more confusing or user just would not find it.

My take on the simplicity matter as this is a library is this: the simplicity of the client code (eg. not make them need to use 3rd party deps or code lines just to use the API) outweights the simplicity of the library.

But as this it your code base just feel free to close this and lets focus our efforts to other matters.

jcrada commented 6 years ago

Oh that's OK. I appreciate very much your contribution. I can see the value of using streams, but I think that given that there are other libraries out there that can very easily retrieve the text as a string from any resource, I prefer people passing the text to fromString. In the end, the engines are not that large anyway to have high performing readings of text. I added the fromFile because many people in the forums kept asking how to do this, and did not seem so straight-forward across programming languages.

Anyway, thanks a lot for this. I will close this PR then.