Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor AgastDetector7_12s_computeCornerScore #3092

Conversation

SunBlack
Copy link
Contributor

Had yesterday again a day full of updating dependencies, so I had again time to be a masochist ;). As last time I used this code to verify I didn't changed behavior.

Most changes are done with help of regular expressions. I'm happy that this exists.

Just in case you want to have fun: My favorite regexe to search for specific code snippets

if \([^\n]*\)\s*return \((?:\([^\(\)]*\) && )*(\([^\(\)]*\))(?: && \([^\(\)]*\))*\);\s*else\s*return \((:?\([^\(\)]*\) && )*?\1(?: && \([^\(\)]*\))\);
  else\n    (\s*)return false;\n\1else\n\1  return false;

Heiko Thiel added 21 commits May 17, 2019 14:05
…gastDetector7_12s_is_a_corner in preparation for next commits
Before:
  if (...)
    return true;
  else
    return false;
Before:
  if (cond1)
    return (cond2)
  else
    return false;

Now:
  return (cond1 && cond2)
Before:
  if (cond1)
    return (cond2)
  else
    return false;

Now:
  return (cond1 && cond2)
Before:
  if (cond1)
    return (cond2)
  else
    return false;

Now:
  return (cond1 && cond2)
Before:
  if (cond1)
    return (cond2)
  else
    return false;

Now:
  return (cond1 && cond2)
Before:
  if (cond1)
    return true
  else
    return (cond2);

Now:
  return ((cond1) || (cond2));
Before:
  if (cond1)
    if (cond2)
      ...
    else
      return false;
  else
    return false;

Now:
  if ((cond1) && (cond2))
    ...
  else
    return false;
Before:
  if (cond1)
    return (cond2)
  else
    return (cond2 && cond3);

Now:
  if (cond2)
    return (cond1 || cond3)
  else
    return false;
Before:
  if (cond)
    return false;
  else
    ...

Now:
  if (!cond)
    ...
  else
    return false;
Before:
  if (cond1)
    if (cond2)
      ...
    else
      return false;
  else
    return false;

Now:
  if ((cond1) && (cond2))
    ...
  else
    return false;
Before:
  if (cond1)
    if (cond2)
      ...
    else
      return false;
  else
    return false;

Now:
  if ((cond1) && (cond2))
    ...
  else
    return false;
Before:
  if (cond1)
    return (cond2)
  else
    return (cond2 && cond3);

Now:
  if (cond2)
    return (cond1 || cond3)
  else
    return false;
Before:
  if (cond1)
    return (cond2)
  else
    return false;

Now:
  return (cond1 && cond2)
Before:
  if (cond1)
    return (cond2)
  else
    return (cond2 && cond3);

Now:
  if (cond2)
    return (cond1 || cond3)
  else
    return false;
Before:
  if (cond1)
    if (cond2)
      ...
    else
      return false;
  else
    return false;

Now:
  if ((cond1) && (cond2))
    ...
  else
    return false;
@SergioRAgostinho
Copy link
Member

Don't forget to rebase this against the current master. We already pushed the fix that gets rid of these out of space errors in the Windows CI.

@SunBlack
Copy link
Contributor Author

? This PR is basing on latest commit in master.

@stale
Copy link

stale bot commented Jul 18, 2019

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

@stale stale bot added the status: stale label Jul 18, 2019
@SunBlack
Copy link
Contributor Author

As currently clang-format will be applied to whole source: Is it possible to merge this PR before? I don't believe I can rebase this as it touches to many lines :/

@stale stale bot removed the status: stale label Sep 25, 2019
@taketwo
Copy link
Member

taketwo commented Sep 25, 2019

It'll take some time until we reach the keypoints module with formatting. But yes, I will try to review this soon.

@SergioRAgostinho SergioRAgostinho added the needs: code review Specify why not closed/merged yet label Sep 25, 2019
@taketwo taketwo removed the needs: code review Specify why not closed/merged yet label Oct 10, 2019
@taketwo taketwo merged commit c887fe8 into PointCloudLibrary:master Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants