2
\$\begingroup\$

Could you please help me to review my first function (modified from an example) in terms of best practice, coding style suggestions and any obvious language mistake/improvements?

unsigned int edit_distance(const vector<string> &s1, const vector<string> &s2)
{
 const size_t len1 = s1.size(), len2 = s2.size();
 vector<vector<unsigned int>> d(len1 + 1, vector<unsigned int>(len2 + 1));
 d[0][0] = 0;
 for(unsigned int i = 1; i <= len1; ++i) d[i][0] = i;
 for(unsigned int i = 1; i <= len2; ++i) d[0][i] = i;
 for(unsigned int i = 1; i <= len1; ++i)
 for(unsigned int j = 1; j <= len2; ++j)
 {
 unsigned int a = d[i - 1][j] + 1;
 unsigned int b = d[i][j - 1] + 1;
 unsigned int c = d[i - 1][j - 1] + (s1[i - 1] == s2[j - 1] ? 0 : 1);
 d[i][j] = std::min( std::min(a,b), c);
 }
 return d[len1][len2];
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jun 12, 2013 at 9:20
\$\endgroup\$
1
  • 3
    \$\begingroup\$ For starters you need a function comment, explain what its doing, use readable variable names. You don't get any performance improvement from minified code. \$\endgroup\$ Commented Jun 12, 2013 at 13:59

1 Answer 1

6
\$\begingroup\$

Do you want unsigned int or do you want std::size_t?

unsigned int edit_distance(const vector<string> &s1, const vector<string> &s2)

It comes down to what you are trying to convey.
The use of string and vector suggests you are doing using namespace std; try not to do this. Prefer to qualify your types: std::string and std::vector.

Personally, I prefer const on the right. But it's only a preference thing.

Only declare one variable per line:

 const size_t len1 = s1.size(), len2 = s2.size();

Come up with better names. d is not very descriptive.

 vector<vector<unsigned int>> d(len1 + 1, vector<unsigned int>(len2 + 1));

I would prefer std::size_t here:

 for(unsigned int i = 1; i <= len1; ++i) d[i][0] = i;
answered Jun 12, 2013 at 14:12
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.