Requesting opinions

[rscarson]

I just released a project I've been working on for a while, and would appreciate any opinions, comments, etc. anyone might have about it. It's a math parsing library, and can be found here:
https://sourceforge.net/projects/ub3rmath/

Thanks in advance,
Richard Carson

[whitenite1]

Tried it out, but came up with an error. Could not #include <unordered_map> from the math library. What library or include, is this from? I'm using Microsoft Visual C++ 2008 Express Edition.

[ne555]

¿where is the source code?

@whitenite1: that's from the new standard.

[rscarson]

Its in a git repo
https://sourceforge.net/p/ub3rmath/code/ci/3abeaf4cfe0b46898fc96e607e449f29309b953f/tree/

I can upload the source as a tarball though, if that's easier

[whitenite1]

Thanks for clarifying that for me ne555. At least I won't keep attempting to use it, til I get the newer compiler. I found the mentioned source code by going to the above https site, but leaving off the last forward slash.

[ne555]

line 357 of Solver.cpp should be delete [] eqParse;
But maybe it would be better if you use std::vector instead.

Don't using namespace std; in headers. It pollutes everything.
You could create a namespace too.

Prefer constants to defines.

[rscarson]

Thank you very much for the response,
I am currently working on those, and in the mean time have updated it to use std::map instead of std::unordered_map. It's a hit on performance, but eliminates the need to support the new standard.

[webJose]

Isn't there a pre-defined symbol for C++11 that you can use to conditionally use either one, assuming they work the same (same method names and such)? If not you can always #define your own.

Users of your library would do:

#define NOCPP11
#include "theheaderfile.h" 

And your header would do something like:

#ifdef NOCPP11
#include <map>
typedef std::map map_type;
#else
#include <unordered_map>
typedef std::unordered_map map_type;
#endif 

And then you use the type map_type everywhere else in your library.

[rscarson]

That's a great idea :D

edit:

Something like this?

#ifdef __GXX_EXPERIMENTAL_CXX0X__
	#include <unordered_map>
	typedef std::unordered_map __map__;
#else
	#include <map>
	typedef std::map __map__;
#endif 

[rscarson]

Also, I was thinking of adding unit conversion filters, like:

celciustofahrenheit
fahrenheittocelcius
metrestofeet
feettometres
inchestocm
cmtoinches
poundstokg
kgtopounds 

Any others I could add? I also am looking to add more constants. All I have is e and pi at the moment.

Thanks again :D

[aquaz]

It's a bad idea to name your symbols with __ at the begining, i think it's used by the compiler and the standard library.

[rscarson]

Any conventions as to how to name a pseudo type like this?

[webJose]

Just add a namespace to your library. Namespaces are exactly for that: Name conflict resolution.

[aquaz]

I see in ConstantesManipulation.cpp that you pass string by value. This is very bad for performances, consider using const& string to avoid copy.
In general, the only types you sould pass by value are primitives types(int, float, double, char, pointers) and very small types designed to be passed by value like iterators and functors.
The only benefit to pass by value is to not have indirection when using the parameter.

[aquaz]

This is personal opinion: in Function.cpp, i find your

ver_tmp == '0' ||
ver_tmp == '1' ||
ver_tmp == '2' ||
ver_tmp == '3' ||
ver_tmp == '4' ||
ver_tmp == '5' ||
ver_tmp == '6' ||
ver_tmp == '7' ||
ver_tmp == '8' ||
ver_tmp == '9')

very ugly, why not put it in a function and use comparison instead?
This function may be useful:

template <char lower, char upper>
inline bool in_range(char letter)
{
    return lower <= letter && letter <= upper;
}

and then you have:

inline bool is_decimal(char letter) { return in_range<'0', '10'>(letter); }
inline bool is_octal(char letter) { return in_range<'0', '8'>(letter); }

and so on...

[rscarson]

Epic! I was looking for a better way of doing this.

[aquaz]

Another idea for performance: in your main loop in Solver.cpp, you do this:

string nextChunk = GetNextChunk(&equation);
/* some code which don't use the string value of nextChunk but its type with GetChunkType */
switch  (getChunkType(nextChunk)
/* ... */

I think it would be better not to treat chunk as string as it doesn't not hold the real information you want to manipulate. In fact, GetNextChunk already knows the data in your chunk because it parse it. So it should return something like a boost::variant<float, string, operator,...> (a variant is a type which can hold data of multiple types and knows about the data of its type). Thus you don't have to call GetChunkType multiple times.
If you don't want to use boost::variant you can easily create your own with an union and an enum for exemple.

[rscarson]

Good idea... I could do something like this maybe:

struct Chunk
{
     string data;
     int chunkType;
};

[aquaz]

Yes that better but use an enum instead of an int.

[rscarson]

I did not know about enum. This is handy.