Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
Home » Developing U++ » UppHub » FTP class and reference example for U++
Re: FTP class and reference example for U++ [message #47859 is a reply to message #47858] Wed, 12 April 2017 23:05 Go to previous messageGo to previous message
Klugier is currently offline  Klugier
Messages: 932
Registered: September 2012
Location: Poland, Kraków
Experienced Contributor
Hello,

I am not tested this package, but i would like to notice small issue with the API design. The problem is that you need to pass too many arguments to the single function, if you want to change single argument like ascii. Let's take a look at below function:
int FtpAsyncGet(const String& remote_file, const String& local_file, const String& host, int port = 21, const String& user = Null,
        const String& pass = Null, Event<int, int, String, int64, int64> progress = CNULL, bool active = false, bool ssl = false, 
        bool ascii = false);


This is the poor design, because the call will look like this:
FtpAsyncGet("remote.txt", "local.txt", "locaclhost, 21, Null, Null, CNull, false, false, true);

Looks terrible. Isn't it? In my opinion the good API has got maximum three parameters - otherwise it becomes hard to use by the client.

We can replace following code with the object approach (This is the example and I highly recommended to over thing this design):
int FtpAsyncGet(const FtpAssyncRequestConfig& config);

FtpAsyncGet(FtpAssyncRequestConfig("remote.txt", "local.txt", "locaclhost.txt").EnableAscii());

class FtpAssyncRequestConfig final
{
public:
    FtpAssyncRequestConfig(const String& remote_file, const String& local_file, const String& host)
        : // initialization ...
    {}

    // Get, Set, Enable interface...

private:
    // private members
};


Much more easier to set the n-th parameter, but require additional work for the library provider.

______________
You could replace NAMESPACE_UPP with namespace Upp { and END_NAMESPACE_UPP with }. The NAMESPACE_UPP approach was deprecated since last release.

______________
The next problem I see in the code is the ordering problem - you should put public class interface at the top then private things (Ftp and DirEntry classes). This is the general rule mentioned in following documentation topic.

______________
This code review aims to improve the code quality.

Sincerely,
Klugier


Ultimate++ - one framework to rule them all.

[Updated on: Wed, 12 April 2017 23:22]

Report message to a moderator

 
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Read Message
Previous Topic: Am I doing something wrong, or is there a bug? FIREBIRD
Next Topic: OpenCV and openCV_Demo copiling error
Goto Forum:
  


Current Time: Fri May 27 08:35:13 CEST 2022

Total time taken to generate the page: 0.01608 seconds