Ideas for how to improve code

wood_brian · 2012-06-14T19:57:25

[font=UsherwoodStd-Medium][font=UsherwoodStd-Medium]"we need a greater appreciation of incremental (engineering) improvements with a relevance to real-world systems. “That’s just engineering, and we’re computer scientists” is an attitude we can’t afford." Bjarne Stroustrup[/font][/font] [font=UsherwoodStd-Medium][font=UsherwoodStd-Medium]I agree with that and am posting here to ask for some ideas on how to improve the code here --[/font][/font] [font=UsherwoodStd-Medium][font=UsherwoodStd-Medium]http://webEbenezer.n...ntegration.html . One area that needs work is settling on a coding convention and adhering to it -- the names of classes/functions/files aren't consistent. Thoughts on that are appreciated, but I hope there will be other ideas. Tia.[/font][/font]

wood_brian

193

Author

May 06, 2012 09:37 PM

Here's a counterpoint for you: Boost is maintained by some of the leading experts in C++ in the world.

Over the years I've received a lot of advice from Boost authors. Some of it has been directed to me and some of it has come from reading the Boost mailing lists and applying something. Originally the only way to use the CMW was via this web interface. (That interface is no longer active/live.) Someone on a Boost list encouraged me to develop an interface that could be easily integrated into build environments. The CMW is a new Boost Serialization library. It has support for some Boost libraries that the serialization library that's in Boost doesn't have. Originally I had very little open-source code... today I have over 5,000 lines of it and growing. I've gotten a lot of helpful input from many highly regarded C++ people: Bjarne Stroustrup, James Kanze, Boost authors, people here and on other forums. And Telastyn noted that the CMW is also free.

Your project is maintained solely by you, and having existed for over 9 years, still has glaring code quality issues by even your own admission.

What do you think people are liable to trust more?
[/quote]
I'm one of the maintainers of the CMW, but not the only one. I don't argue that the CMW has no room for improvement. That's why I started the thread. The point is the serialization library that's in Boost is losing momentum. I wouldn't be working on this if I didn't believe there was a changing of the guard taking place. The older approach worked OK for years, but now there's a better (more portable and sustainable) way to do it. We're out here proving it to people. The remarks about how the serialization library in Boost hasn't been well maintained is getting at how sustainable that model is. A business model matters if you want a helpful partner that will be here for as long as your project lasts. Is Google going to release a version of Google Docs that you install locally? Is Microsoft going to stop developing Office 365?

wood_brian

193

Author

May 06, 2012 10:44 PM

Your project is maintained solely by you, and having existed for over 9 years, still has glaring code quality issues by even your own admission.
[/quote]

A couple more thoughts on this. I agree with those who say, "C++ isn't very good, but it is better than the alternatives." And I think Stroustrup made the comment, "now we have to hurry up and fix everything" when C++ started getting popular. I don't think C++ has fixed everything yet, but it has improved a lot over the years. I believe the CMW is also on the right path -- getting better over time.

the_edd

2,109

May 07, 2012 02:35 PM

Something is wrong with getaddrinfo if it doesn't set res and it doesn't fail.

Assertions check and document your understanding and assumptions. So this is a good place for one, I'd say.

http://www.mr-edd.co.uk
http://bitbucket.org/edd

wood_brian

193

Author

May 13, 2012 07:45 PM

Recently I noticed the last line in an if matched the last line in an else here:



void

cmwAmbassador::mediateResponse (transactions_t::iterator itr)

{

  bool safe_sending = true;

  try {

	localsendbuf.sock_ = itr->sock;

	bool reqResult;

	remoteMessages.Receive(cmwBuf, reqResult);



	if (reqResult) {

	  changeDirectory(itr->path.c_str());

	  ::std::vector<File> outputFiles;

	  remoteMessages.Receive(cmwBuf, outputFiles);



	  save_lastruntime(itr->filename, itr->time_in_seconds);

	  safe_sending = false;

	  localMessages.Marshal(localsendbuf, true);

	  localsendbuf.Flush();

	} else {

	  ::std::string errorMsg;

	  remoteMessages.Receive(cmwBuf, errorMsg);

	  safe_sending = false;

	  localMessages.Marshal(localsendbuf, false, errorMsg);

	  localsendbuf.Flush();

	}

  } catch(::std::exception const& ex) {

#ifdef SYSLOG_AVAILABLE

	syslog(LOG_ERR, "mediateResponse: %s", ex.what());

#endif

	if (safe_sending) {

	  localMessages.Marshal(localsendbuf, false, ex.what());

	  localsendbuf.Flush();

	}

  }

}

... and I factored it out. I felt certain the new version would be the same size as it is such a simple thing. I was surprised when the size of the executable decreased by 88 bytes using gcc 4.7.0 20120314. Using gcc 4.8.0 20120429 the size decreased by 56 bytes. I'm using -O3 in both cases. This isn't some tricky thing and these are some recent compilers. I thought twice about even doing this as I was so sure the compiler would be able to catch it.

Antheus

2,410

May 13, 2012 07:56 PM

HP drivers I use are 370MB. Radeon drivers are 250MB.

I have 16GB of RAM and 3TB of disk.

Just saying.

For machines where 30 bytes matters, C++ is absolutely the wrong choice, precisely due to the hidden magic and "bloat", compared to C.

wood_brian

193

Author

May 20, 2012 09:29 PM

For machines where 30 bytes matters, C++ is absolutely the wrong choice, precisely due to the hidden magic and "bloat", compared to C.

Someone in another forum pointed out the local objects being destructed -- not deep magic, but that explains part of it. I've now made those static local objects. After doing that the size of the executable is 28 bytes larger when the line is repeated in both branches. Perhaps that's all that can be done and I'll just chalk the 28 bytes up to a weakness in the compiler.



void

cmwAmbassador::mediateResponse (transactions_t::iterator itr)

{

  bool safe_sending = true;

  try {

		localsendbuf.sock_ = itr->sock;

		bool reqResult;

		remoteMessages.Receive(cmwBuf, reqResult);



		if (reqResult) {

		  changeDirectory(itr->path.c_str());

		  static ::std::vector<File> outputFiles;

		  outputFiles.clear();

		  remoteMessages.Receive(cmwBuf, outputFiles);



		  save_lastruntime(itr->filename, itr->time_in_seconds);

		  safe_sending = false;

		  localMessages.Marshal(localsendbuf, true);

		  localsendbuf.Flush();

		} else {

		  static ::std::string errorMsg;

		  remoteMessages.Receive(cmwBuf, errorMsg);

		  safe_sending = false;

		  localMessages.Marshal(localsendbuf, false, errorMsg);

		  localsendbuf.Flush();

		}

  } catch(::std::exception const& ex) {

#ifdef SYSLOG_AVAILABLE

		syslog(LOG_ERR, "mediateResponse: %s", ex.what());

#endif

		if (safe_sending) {

		  localMessages.Marshal(localsendbuf, false, ex.what());

		  localsendbuf.Flush();

		}

  }

}

Cornstalks

7,033

May 20, 2012 09:59 PM

[quote name='Antheus' timestamp='1336939010' post='4939864']
For machines where 30 bytes matters, C++ is absolutely the wrong choice, precisely due to the hidden magic and "bloat", compared to C.

Someone in another forum pointed out the local objects being destructed -- not deep magic, but that explains part of it. I've now made those static local objects. After doing that the size of the executable is 28 bytes larger when the line is repeated in both branches. Perhaps that's all that can be done and I'll just chalk the 28 bytes up to a weakness in the compiler.
[/quote]
Honestly, I'm not sure why you even care about 28 bytes. Improving code and minimizing executable size don't really go hand in hand, especially if it's a matter of just a few bytes. If you're really, seriously, critically concerned about shaving off a few bytes, you'd be pulling out ugly code hacks anyway.

[size=2][ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

wood_brian

193

Author

May 20, 2012 10:07 PM

try/catch(...) anywhere but at an extremely high level always worries me. Any instance of throw; immediately worries me even more. The combination is egregious. You should really reconsider your approach to exceptions. Also, I haven't confirmed this deeply, but I suspect that most of your code is highly unsafe in the presence of thrown exceptions.

I grepped through Boost 1.49 and try/catch(...) and throw; are both used in hundreds of places. You've mentioned Boost in this thread in positive terms so am pointing out that others are using the same functionality in a similar context.

Cornstalks

7,033

May 20, 2012 10:13 PM

[quote name='ApochPiQ' timestamp='1336085463' post='4937241']

try/catch(...) anywhere but at an extremely high level always worries me. Any instance of throw; immediately worries me even more. The combination is egregious. You should really reconsider your approach to exceptions. Also, I haven't confirmed this deeply, but I suspect that most of your code is highly unsafe in the presence of thrown exceptions.

I grepped through Boost 1.49 and try/catch(...) and throw; are both used in hundreds of places. You've mentioned Boost in this thread in positive terms so am pointing out that others are using the same functionality in a similar context.
[/quote]
Boost can be configured to use no exceptions... See boost/serialization/throw_exception.hpp, seeing as that seems to be a relevant comparison to your project.

[size=2][ I was ninja'd 71 times before I stopped counting a long time ago ] [ f.k.a. MikeTacular ] [ My Blog ] [ SWFer: Gaplessly looped MP3s in your Flash games ]

wood_brian

193

Author

May 20, 2012 10:18 PM

Honestly, I'm not sure why you even care about 28 bytes. Improving code and minimizing executable size don't really go hand in hand, especially if it's a matter of just a few bytes. If you're really, seriously, critically concerned about shaving off a few bytes, you'd be pulling out ugly code hacks anyway.

Well, I'm happy about how this has turned out -- noticing that I can make the variables static. I'm not very concerned now about the 28 bytes, but if I hadn't brought it up, I probably wouldn't have thought about making the variables static.

Ideas for how to improve code

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Ideas for how to improve code

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines