|
Aside from the fact that the code doesn't do anything useful, the really serious problems with this class are:
- It will leak in normal operation (I use the term "operation" loosely here as the class doesn't do anything)
- It isn't exception safe or neutral
To fix these you're going to need a destructor, a copy constructor (or a private declaration of a copy constructor) and a swapping assignment operator (or a private declaration of an assignment operator).
In addition (ignoring the questionable sanity of storing an int like that) the implementation could be tightened up to use an initialiser list.
The idea here is to make a class that behaves like a object of a type built into the language. The structure your class needs to achieve this is known in some circles as "The Orthodox Canonical Form" and the first place I saw it mentioned and explained properly was in "Advanced C++: Programming Styles and Idioms" By Jim Coplien back in the early 90s. Other authors have picked up the term since then so a search for it might give you some more background.
Cheers,
Ash
|
|
|
|
|
I support this answer, this is more reasonable.
Величие не Бога может быть недооценена.
|
|
|
|
|
Oh, sure, if you want to get all technical. 
|
|
|
|
|
Building on some of the already posed answers:
As mentioned before, you need a destructor to make sure pi is released.
Moreover, if you provide no Copy constructor or assignment operator, the compiler will create these for you. Both will simply copy pi over without checking, und thus also produce a memory leak. This means you need to define both a copy constructor and an assignment operator yourself, to prevent memory leaks from inadvertent copying!
Another, minor, issue is that you didn't think to initialize the integer pi is pointing to.
I will ignore the apparent uselessness of the class in it's current state, assuming that it's going to be expanded later. Going with the above, I would rewrite this to sth like that:
class A {
int* pi;
public:
int* getpi() {
if (0 == pi)
pi = new int();
if (pi != 0)
*pi = 0;
return pi;
}
A() : pi(0) {}
A(const A& other) : pi(0) {
*getpi() = *other.pi;
}
~A() { delete pi; }
A& operator=(const A& other) {
*getpi() = *other.pi;
return *this;
}
}
Note that you could also implement a shallow copy by just redirecting pi to wherever the other object's pi is pointing, but if you do that, you need some mechanism such as reference counting to prevent deleting an int object while oter instances are still pointing to it!
Note also that I only programmed the happy case with respect to allocation: if getpi() ever fails to allocate pi (and thus returns 0), then the call to the copy constructor or assignment operator will throw a NULL pointer exception. You might want to catch that, especially in the constructor.
[edit]Made getpi() public to resolve the inaccessibility issue, and added initializion of *pi[/edit]
modified on Monday, January 31, 2011 6:02 AM
|
|
|
|
|
I expect the standard specifies that whitespace within macro replacement text be reduced to a single space, but I am experimenting with something that requires otherwise.
Does anyone know of a C preprocessor with an option to retain whitespace within the replacement text?
After all, most will allow you to retain comments intact.
|
|
|
|
|
As far as I know, none do. And there is a reason for that. You don't need to, and it makes absolutely no difference to the code.
Quoted strings inside macros retain their white space, for obvious reasons.
What are you trying to achieve with this?
(Maintaining the # symbol would be pretty awesome tho)
EDIT: They maintain the fact that "there is white space there" because that often matters, but they don't necessarily remember how much
|
|
|
|
|
Andrew Brock wrote: there is a reason for that
Not a very compelling one, in my opinion.
Andrew Brock wrote: You don't need to
Ordinarily no, but ordinarily you don't need to include comments either, yet you can if you choose to.
Andrew Brock wrote: it makes absolutely no difference to the code.
It does to what I'm trying now.
Andrew Brock wrote: (Maintaining the # symbol would be pretty awesome tho)
That can be done with a second macro. I've done that for PRO*C, and in what I'm trying to do now.
# define __second_pass__ #
__second_pass__ include <stdio.h> /* This will get included on the second pass through the preprocessor, not the first */
But, what I'm trying to do now is have a macro produce a directive for the second pass, and for that I need linefeeds. It's really not worth the effort and only I would try, but as long as preprocessors allow retaining comments, why not also allow retaining whitespace in macroes?
|
|
|
|
|
PIEBALDconsult wrote: Not a very compelling one, in my opinion.
Macros exist to provide some specific functionality in C and C++. They don't exist to meet every possible known need. Myself I am comfortable with the notion that the authors of those languages and the numerous others who contributed to the ANSI specifications did in fact give due consideration to what it should do an what it shouldn't do. Given the extensive years of scrutiny and use that resulted from that I would suspect that your "compelling one" isn't very.
I am pretty sure that the creators of C and C++ were comfortable with the notion that if someone wanted to create their own language then they should in fact do exactly that.
And there are in fact other macro compilers. I suspect that the following provides quite a bit of functionality. And since one can in fact modify what it does you can do whatever you want with it.
http://www.gnu.org/software/m4/[^]
However, myself would not want to maintain code where someone thought that they needed to use clever tricks via the macro to achieve something. The vast majority of time clever tricks increase the cost of code maintenance.
|
|
|
|
|
jschell wrote: your "compelling one" isn't very.
I didn't say there was one.
jschell wrote: other macro compilers
I hadn't considered that, thanks.
jschell wrote: clever tricks increase the cost of code maintenance
Absolutely, and this is kinda to prove that point.
|
|
|
|
|
PIEBALDconsult wrote: I didn't say there was one.
I apologize. Your post is clear but somehow I managed to read something into it which wasn't there at all.
|
|
|
|
|
I have an application which work fine if it is setup inside the folder with English characters for example "myapplication" but if I does not run when I try to install it inside in a folder which has folder name like "выполнятЬ" however installation does not create problem.
The problem comes with flash player activeX. I have integrated swf inside a dialog. I calls it's function like below
try
{
_bstr_t response = flashDlg.flashPlayer->CallFunction (query);
}
catch(...)
{
logManager.Print(LL_INTERR, "Error while sending command to swf..");
return false;
}
First 3 times it calls the function without problem but after that it throws error I traps into catch. Any suggestion or guess what can be the problem.
modified on Saturday, January 22, 2011 6:56 AM
|
|
|
|
|
You need to use (preferably) Unicode otherwise multi-byte strings to support the extended character set.
This is in the General Project Options currently as Character Set = Not Set. Change this to Use Unicode Character Set
Then you will need to add the _T() macro around all of your strings like logManager.Print(LL_INTERR, _T("Error while sending command to swf.."));
|
|
|
|
|
I am already using that option "Use Unicode Character Set". The problem is uncertain this type of folder name works fine in my machine but does not work in other machines and my PM always says  "Everything runs into your machine why it does not run in other machines"
One thing is sure that it is somehow related to language issue because it works fine in all machines if they have folder name in English.
Another strange thing is that swf loads without any problem. Initial function call goes fine but later on it does not work even though data as well as function name both are same.
|
|
|
|
|
If I am understanding this correctly, it installs ok into the folder, but wont run from it.
If that is correct:
The English language is in the ASCII character set. The Unicode and Multi-Byte character sets are just supersets of the ASCII character set.
Having the Unicode character set enabled is only half the story. You need to use the Unicode functions.
I am certain if you are using the Unicode functions, then it will work.
For functions like fopen are ASCII, you need to use it like FILE *pFile = _tfopen(_T("C:\\выполнятЬ"), _T("w"));. Also not that the source code file needs to be saved in Unicode format if you use non-english strings in it like that one. Visual Studio will prompt you if this needs to happen.
For the Win32 API functions, they will automatically change over to use the Unicode versions unless your function names finish with a capital A
If it aren't understanding it correctly, and it doesn't install into the folder, then try a different setup program.
|
|
|
|
|
Yes you got it right. It is installing perfectly but not running. However problem is not in writing files or opening file. It is between Flash ActiveX and my wrapper function who actually calls it.
Here is the implementation of CallFunction()
inline _bstr_t IShockwaveFlash::CallFunction ( _bstr_t request )
{
BSTR _result = 0;
HRESULT _hr = raw_CallFunction(request, &result);
if (FAILED(_hr)) _com_issue_errorex(_hr, this, __uuidof(this));
return _bstr_t(_result, false);
}
It FAILs and then goes for _com_issue_errorex
If my parameter are wrong then it should not work even if it was installed inside English folder. Function Parameters does not have non English chars.
|
|
|
|
|
Find the value of _hr when the it has failed.
You can do 1 of the following:
1. Put a breakpoint on the line if (FAILED(_hr)) _com_issue_errorex(_hr, this, __uuidof(this));
2. Print a message with TRACE("%u", _hr); to the "Output" window in Visual Studio if you are using MFC or ATL
3. printf("%u", _hr); if it is a console app
4. Show it in a MessageBox
Let me know what the code is, or see this page on MSDN[^] if you would like to have a shot at decoding the error yourself.
|
|
|
|
|
I added a break point there to check HRESULT value. It was E_FAIL.
|
|
|
|
|
Well, that's a great deal of help. A generic error code. 
Perhaps checking the value of query may help, see if it isn't sending anything strange
Other than that, the best I can do then is suggest running ProcMon[^] and filter by your application and Filesystem activity to see if there is any errors when accessing the directory.
Other than that check the documentation.
|
|
|
|
|
Andrew Brock wrote: You need to use the Unicode functions.
I am certain if you are using the Unicode functions, then it will work.
I don't think either of these statements is true. The use of Unicode or ANSI inside a program is not dependent on the character set or language of the platform. If this were the case then I suspect thousands of programs would not run.
I must get a clever new signature for 2011.
|
|
|
|
|
At that stage (until he modified the question) I thought the problem was something along the lines of opening a file that lived inside a folder with a non-english name, then the program crashing as a result of reading from a file that isn't open.
Windows is Unicode. Most functions (in the Win32 API) that have an ASCII version convert the ASCII string to Unicode then call the Unicode version of the function.
Having said that, how do you specify a directory name (in Russian) that only exists within extended character sets (such as Unicode) with only ASCII. You can't.
The OP has since modified the question and has better defined his problem and the location of it, which now makes my answer irrelevant.
|
|
|
|
|
No problem, I just thought you were going off on a bit of a tangent. What you say here is correct, and obviously you cannot specify extended character file names in ASCII.
I must get a clever new signature for 2011.
|
|
|
|
|
You need to capture more information in your catch block, or use the debugger to trap it and see what is going on in the CallFunction() method; perhaps the parameter is incorrect.
I must get a clever new signature for 2011.
|
|
|
|
|
I was unsure what type of error it was throwing so I used ... in catch block. Debugger is not able to help here. By debugging, I have checked for the expiry of the flash object but it was alive so no problem there. Even parameters are also right.
|
|
|
|
|
I think Andrew has made some useful suggestions above. You need to display the actual error code in your catch block; in your case you are just printing a rather unhelpful message.
I must get a clever new signature for 2011.
|
|
|
|
|
This CallFunction is inside flash9c.tli file. Whenever I write any helpful statement there and compiles the code then it automatically reverts back. I am not able to put any statement there. I think this file generates automatically at compile time.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
