I wrote a program for my C++ class (it finished a few weeks ago) and my instructor said a comment I didn't really understand (he didn't elaborate on the comment). He said that it seems to be written in Java or seems Java-like.

I'm not going to post whole thing here (too long) but he did say the comment on a particular piece of code, wherein we were to do the ff.: (Paraphrased)

Given a list of names, allow the user to enter a search term. Return all strings which have that search term as a substring. Write your own code to determine if a string is a substring of another.

Here's what I came up with:

vector<string> Directory::searchByName(string nameArg) 
 vector<string> matches;

 for (string r : records) 
 int pos = 0;
 int last = r.length();

 bool isFound = false;
 do 
 int x = pos, a;
 for (a = 0; a < nameArg.length() && x < last; a++, x++) 
 if (toupper(nameArg[a]) != toupper(r[x])) break;
 
 if (a == nameArg.length()) 
 matches.push_back(r);
 isFound = true;
 break;
 

 int temp_pos = pos;
 pos = r.find(toupper(nameArg[0]), temp_pos + 1);
 if (pos == string::npos) 
 pos = r.find(tolower(nameArg[0]), temp_pos + 1);
 
 while ( !isFound && pos != string::npos && pos < last);
 

 return matches;

The code runs (and could be optimized, I admit) but I just don't understand that comment. Did I do something wrong or unconventional in terms of C++ in my syntax? If so, what is it and how can I improve it?

Somebody asked me to include code that can be compiled. We were also practicing the three file format thing so I'm not sure how to do that here.

Full code:

main.cpp

#include "Directory.h"
#include <iostream>
using namespace std;

int main() 
 Directory dir;
 string searchTerm;
 // dir.lazyInit();
 // dir.saveToFile("dat.dat");
 // dir.displayAllRecords();
 dir.loadDirFromFile("dat.dat");

 cout << "This program searches for records that " << endl
 << "matches a full name or a partial name." << endl
 << "Please enter a name as prompted." << endl << endl;

 cout << " Search? ";
 getline(cin, searchTerm);
 cout << endl;

 vector<string> results = dir.searchByName(searchTerm);
 if (results.size() == 0) 
 cout << "We're sorry. There are no matches." << endl;
 else 
 cout << "The following records are found: " << endl;
 for (string r : results) 
 cout << " " << r << endl;
 
 

 return 0;

Directory.h

#ifndef HOMEWORK_CH12_QN11_DIRECTORY_H
#define HOMEWORK_CH12_QN11_DIRECTORY_H

#include <vector>
#include <string>

class Directory 

private:
 std::vector<std::string> records;

public:
 void lazyInit();
 void displayAllRecords();
 std::vector<std::string> searchByName(std::string);

 void saveToFile(std::string);
 void loadDirFromFile(std::string);

;

Directory.cpp

#include "Directory.h"
#include <iostream>
#include <fstream>
using namespace std;

void Directory::lazyInit() 
 records = 
 "Hoshikawa Tanaka, 678-1223",
 "Joe Looney, 586-0097",
 "Geri Palmer, 223-8787",
 "Lynn Lopez, 887-1212",
 "Holly Gaddis, 223-8878",
 "Sam Wiggins, 486-0998",
 "Bob Kain, 586-8712",
 "Tim Haynes, 586-7676",
 "Warren Gaddis, 223-9037",
 "Jean James, 678-4939",
 "Ron Palmer, 486-2783"
 ;


void Directory::displayAllRecords() 
 for (string s : records) 
 cout << s << endl;
 


 vector<string> Directory::searchByName(string nameArg) 
 vector<string> matches;

 for (string r : records) 
 int pos = 0;
 //int last = r.find(',');
 //if (last == string::npos) last = r.length();
 int last = r.length();

 bool isFound = false;
 do 
 int x = pos, a;
 for (a = 0; a < nameArg.length() && x < last; a++, x++) 
 if (toupper(nameArg[a]) != toupper(r[x])) break;
 
 if (a == nameArg.length()) 
 matches.push_back(r);
 isFound = true;
 break;
 

 int temp_pos = pos;
 pos = r.find(toupper(nameArg[0]), temp_pos + 1);
 if (pos == string::npos) 
 pos = r.find(tolower(nameArg[0]), temp_pos + 1);
 
 while ( !isFound && pos != string::npos && pos < last);
 

 return matches;
 

void Directory::saveToFile(string filename) 
 fstream file(filename, ios::out 

void Directory::loadDirFromFile(string filename) ios::binary);

 if (!file) return;

 while(file.good() && file.peek() != EOF) 
 int l = 0;
 file.read(reinterpret_cast<char *>(&l), sizeof(l));
 file.read(buffer, l);
 buffer[l] = '';

 records.push_back(static_cast<string>(buffer));
 

 file.close();

The code you posted is good C++, although, it does show some traits from someone who programmed Java. I would accept it in a C++ code-base in the state it is in.

Braces

The brace style you are using is an exact match with the java style guide.
I can only expect that your professor expected something different.
However, out of all, I can't tell which would be most c++.

void function() 
 // Java


void function()

 // Allman

void function()
 
 // Whitesmiths
 

String.length()

In java, getting the length/size of a string is done via the length() method.
In C++, both length() and size() exist. In my experience, I've only seen the last one being used.

int last = r.size();

Passing arguments by value ain't cheap

If I have to compare C++ with Java, I would state that java passes all arguments as something that looks like an std::shared_ptr. In C++, the arguments are copied unless you accept them by reference.

Instead I would suggest to make the signature:

vector<string> Directory::searchByName(const string &nameArg)

Similar for the ranged based for:

for (const string &r : records)

using namespace std;

Don't.

Of what use is Directory?

This class is just a collection of functions. It serves no real purpose: It doesn't really bring any data and logic together. All it does is needlessly limit the access to the data structure with your records.

In other words: It doesn't protect any(*) invariants. Why hide the std::vector in it?

(*): Actually it does, of course, but I'm relatively sure that's not needed: You cannot add, sort, remove, ... any records.

I'd guess this is what your instructor meant with the Java-ish comment. (Because in Java, everything needs to be inside some class).

My suggestion; implement the following "interface" (as free functions):

std::vector<std::string> readRecordsFromFile(std::string filename);
// return empty on error or throw exception
// or: bool readRecordsFromFile(std::vector<std::string> &, std::string)

bool writeRecordsToFile(std::string filename);
// or ditch the return type and throw an exception on error

std::vector<std::string> findRecordsWithSubstring(
 std::vector<std::string> const & records,
 std::string const & substring);

And then, to implement the last one, use std::copy_if and a predicate where you ...

Write your own code to determine if a string is a substring of another.

bool isSubstringOf(std::string const & part, std::string const & whole);

Sanitize / Secure input

const int BUFFER_SIZE = 256;
static char buffer[256];

// ...

 int l = 0;
 file.read(reinterpret_cast<char *>(&l), sizeof(l));
 file.read(buffer, l);
 buffer[l] = '';

What if l is read as 34544? If you have a fixed size buffer, then at least protect against overflow! Also use the BUFFER_SIZE instead of a magic number!

Tests

Add unit tests to ensure functions you wrote (the search function, the substring test, ...) work the way you intended. Use golden files ("golden tests") to ensure your input / output works as expected.

Minor

std::endl flushes the stream

Using std::endl means: Write a n and flush the stream. The later is a potentially costly operation. So when writing text(*) to a stream, use literal n for all newlines except the last.

(*) Unless you write lots of text and need it to be refreshed on "screen" repeatedly. Think of "following" logging output.