Code review comment for lp://staging/~bijanbina/ginn/GIR

First, Stephen has noted many issues with the changes. I agree with all of them, so it makes sense to have them fixed before doing any further review of the code.

> 1: if you think renaming is getting you to trouble so i must do what to port
> to c++

It's not the renaming that is the problem. If the renaming provided some benefits it would be acceptable. However, the renaming here is to allow for C++ compilation. Thus, there needs to be worthwhile C++ usage or else we could just leave it as C.

This hits at a big issue with the changes. I don't think you fully understand C++, how to use it properly, and how to make code better and more readable with it. Ginn could benefit greatly from being converted to C++. However, these changes do not make the code better.

> 2: No why you think so

See the preview diff below. There are lines like:

=== modified file 'COPYING' (properties changed: -x to +x)

> 3: i remove it if the code accepted

The way to fix this is to remove it in your branch, commit the change, and then push it to this branch on launchpad.net. That will update the merge proposal.

> 4: the comment is not part of code! please review the code not comment!

Comments are just as important as the code. Style matters in keeping things readable. Thus, we review comments too.

> 5: can you make instance in my code
> 6: can you make instance in my code

These are very basic and fundamental concepts of how to write C++. You are asking us to teach you C++. Unfortunately, we don't have enough time to do that. Instead, please try to learn C++ best practices by reading books or searching online for information.

> 7: it's the c++ advantage unlike C it delete them automatically

No, that is incorrect. You must delete C++ objects created with new.

> 8: it's only on some classes and it is because if we change it need to
> recreate a code fuly again!!

This is part of the fundamental aspects of C++. It needs to conform to normal C++ coding standards.

> 9: if you use a very long function name you made the developer get to pitfall
> it's the best way to simple the develop process and have various advantage

There are many possible ways to name functions, with many different reasons for why they are named a certain way. Normally, you should try to stay consistent with the existing code, and ensure that the names are obvious. Adding 'G' and 'D' to the beginning of function names does not have an obvious meaning.

> 10: it use to create a local copy of ginn object that is load for device .it's
> show the duty fully

You can name it "ginn" or something more descriptive. "local" is too generic for a variable name.

> 11: i like this way;also adding header not make your software slow!

You can add a header, that's not the issue. The issue is that the name of the header is too generic. The name needs to give some idea as to what is defined in the header.

> 12: it's an static error ,add error_t is for just through of the compiler
> error. also use error_t for dynamic error.

I don't think you understand how exceptions work. Please look up C++ exception handling.

> 13: the comment is not part of code! please review the code not comment!

This has nothing to do with comments. It's a style issue that can easily be fixed.

I want to thank you for your efforts and your work so far. However, it is clear that you don't know how to use the C++ language properly. I think it would be better for you to learn more about C++ and then come back to help improve ginn.

Because I feel the code needs more than just a few changes, I am disapproving the merge proposal.