lordgamez commented on code in PR #1669: URL: https://github.com/apache/nifi-minifi-cpp/pull/1669#discussion_r1341548877
########## libminifi/include/utils/file/ArchiveUtils.h: ########## @@ -0,0 +1,31 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include <memory> +#include <map> +#include <string> + +#include "io/ArchiveStream.h" +#include "utils/expected.h" +#include "io/BufferStream.h" + +namespace org::apache::nifi::minifi::utils::archive { + +nonstd::expected<std::shared_ptr<io::BufferStream>, std::string> createArchive(std::map<std::string, std::unique_ptr<io::InputStream>>& files, const std::shared_ptr<core::logging::Logger>& logger); Review Comment: Sorry, what I actually meant is that I don't want to go into philosophical discussions _here_ as it is not closely related to this PR, but I agree it's good to have debates (in a separate thread :)) I think in this specific case having a function that takes a map of files, returns a stream, and used two different places in the codebase can be extracted and called as a utility. It is a utility function that has a general purpose for two modules outside of the main domain of those modules. I would not overgeneralize the interface until we would need third use case that requires this interface to be refined. I would rather have our use cases refine our interfaces and only develop our code for the current requirements than trying to predict what future requirements will be. There was a talk about this years ago that optimizing for code reuse is overrated as most `generic` purpose classes and functions are used 2-3 times in their lifetime and making them too generic usually come with unneeded compexity. Unfortunately I can't find that talk, but I think [this blog post](https://medium.com/@scott.boring.sb/dont-write-reusable-code-a857e925b683) summarizes it well. "Reuse is an outcome, not a design pattern." On the other hand maybe I was wrong calling it a "util", and of course we have different concepts of what a "util" is. In the project we also have some naming issues as we have too many "Utils" in the code already. We may have to think of another way to group our (somewhat) general functionalities that may not have to be globally generic, but only generic in the domain of a specific module. [Kevlin Henney](https://twitter.com/KevlinHenney/status/715093313301454849) had a whole talk about this that the problem with `util` modules that usually people put classes, functions in `utils` when they cannot really be categorized and it is mostly just a dumping ground for everything. Maybe we could think about reorganizing `libminifi/utils` to separate encapsulated libraries along their functionalities. (It may solve the problem of having to rebuild everything in libminifi when a `util` is changed) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
