1

Usually when i want to check if more input stored in a multiple strings are not empty i follow this simple approach:

std::string fieldA = "";
std::string fieldB = "";
std::string fieldC = "Hello";

Now, i can check for all:

if ( fieldA.empty() || fieldB.empty() || fieldC.empty() )
  std::cout << "Oh oh.. one or more fields are empty << std::endl;

But it would be nice to know which fields are empty, then, i can write:

if ( fieldA.empty() )
  std::cout << "fieldA is empty" << std::endl;
if ( fieldB.empty() )
  std::cout << "fieldB is empty" << std::endl;
if ( fieldC.empty() )
  std::cout << "fieldC is empty" << std::endl;

But in this way i can discover that fieldA is empty but not the fieldB and in this example i have only three fields, but with more fields?

What is the best practice to managing the control of many strings and locate the empty string?

carbeman
  • 137
  • 1
  • 9
  • 8
    Learn to use arrays or container classes instead of individual string variables. Then the rest becomes easy. – PaulMcKenzie Nov 06 '16 at 06:54
  • Variable names are not availble to the code at runtime in the normal course of events. If you want to associate names with values at runtime you need some kind of map. – user207421 Nov 06 '16 at 09:43
  • Consider that variable names in your code and text seen by the user should be two different things, anyway. Your variable might be called `fieldFirstName`, but the user should see something like "First Name". – Christian Hackl Nov 06 '16 at 10:13

4 Answers4

2

PaulMcKenzies comment is the one you should follow. But assuming your example is an over simplification of your code, and you can't put all your variables in an array, I think you can be excused if you use a little macro to do stringification for you:

#define PRINT_IF_EMPTY(var) \
do { \
  if (var.empty()) \
    std::cout << #var " is empty" << std::endl; \
} while(0)

You can now replace your code with this:

PRINT_IF_EMPTY(fieldA);
PRINT_IF_EMPTY(fieldB);
PRINT_IF_EMPTY(fieldC);
StoryTeller - Unslander Monica
  • 165,132
  • 21
  • 377
  • 458
  • Honestly, no, I don't think this can be excused. I'd certainly fight it in any code review! :) – Christian Hackl Nov 06 '16 at 10:10
  • @ChristianHackl, you'd hold little sway if such macros are part of a debugging utility and get conditionally compiled away :) – StoryTeller - Unslander Monica Nov 06 '16 at 10:15
  • The point is not whether they get compiled away or not (in fact, I would *not* want to remove valuable debugging information in release builds, where it is needed most, but it shouldn't just be written to stdout, of course). The point is that they make the code harder to read. The macro also does not indicate that it only exists for debugging, nor that it is compiled away with certain compiler settings. Additionally, it uses `std::endl`, which should likely be `'\n'`, but that's a minor nitpick. – Christian Hackl Nov 06 '16 at 10:22
  • @ChristianHackl, I'd say you are nitpicking this macro example. The OP asked a simple question about removing copy-pasta code. Now, granted, I don't know if it is for debugging purposes; the answer is only about the how. If you believe this is a technically poor answer, or a production quality answer should be given, feel free to down-vote and provide the example. – StoryTeller - Unslander Monica Nov 06 '16 at 10:26
  • I'm trying to keep a beginner from using macros, that's true, but that's not enough reason to downvote. It's technically correct, it just brushes away all the proven and tested guidelines against macro usage in C++. I'd honestly prefer the "copy-pasta code", which is harder to write but far easier to read. – Christian Hackl Nov 06 '16 at 10:38
  • @ChristianHackl, those "proven and tested guidelines" are pretty dogmatically shouted here on SO, without any consideration of merit. Kind of like a perfectly valid example of `goto` in the Linux kernel will be swamped by links to "GOTOs Considered Harmful" by people who never read the actual article or thought about it. If you happen to prefer copy-pasta in [this case](http://stackoverflow.com/a/40362099/817643) as well, we'll continue to do code review differently. – StoryTeller - Unslander Monica Nov 06 '16 at 10:44
  • Well, yes, I do believe that a template could as well do that job. Something like `template struct CheckIteratorValueType { static bool const value = std::is_same::value; };`, with a helper template to get rid of the `::value` as well. You'd use it like `static_assert(check_iterator_value_type, "my_set iterator value type must be double");`. I think this is clearer: 1.) The `static_assert` is immediately visible for the reader, 2.) The message is easier to customise. – Christian Hackl Nov 08 '16 at 15:57
  • @ChristianHackl, 1) The `static_assert` can be visible by simply renaming the macro `STATIC_ASSERT_ITERATOR_VALUE_TYPE` 2) The idea is to avoid repetition, the template loses points on that front 3) It loses points for the verbosity involved in just using the construct. 4) You have to repeat the type name in both the template *and* the message. For shame. – StoryTeller - Unslander Monica Nov 08 '16 at 19:17
0

It's better to use container (std::vector, std::list, std::array e.t.c) for similar type of data.

Let's assume you have stored your std::string in std::vector.

std::vector<std::strting> data;
// Now you have to insert std::string in std::vector.

for( int i = 0; i < data.size(); i++) {
    if(data[i].empty())
        std::cout << "field" << (char)i+65 << " is empty \n";
}
Shravan40
  • 8,922
  • 6
  • 28
  • 48
0

You should use a separate FieldValidator class. With that, your code should look like this:

FieldValidator val;
val.requireNotEmpty("field1", field1);
val.requireNotEmpty("field2", field2);

The idea is that all the validation state is kept in one place. I'm sure such a class already exists somewhere since you are not the first to solve this task.

Roland Illig
  • 40,703
  • 10
  • 88
  • 121
0

You could use a for-range loop over an initializer list, if printing which field is empty is not mandatory and if you prefer the fields to be distinct variables :

for (const std::string str : {fieldA, fieldB, fieldC})
{
    if (str.empty())
    {
        std::cout << "One or more fields are empty." << std::endl;
        break; // Break out of the for loop
    }
}
asu
  • 1,875
  • 17
  • 27