The "IT DOESN'T WORK!" thread

A place to discuss the implementation and style of computer programs.

Moderators: phlip, Moderators General, Prelates

ThomasS
Posts: 585
Joined: Wed Dec 12, 2007 7:46 pm UTC

Re: The "IT DOESN'T WORK!" thread

Postby ThomasS » Sat May 14, 2011 9:37 am UTC

In regards to the exact question that was asked, if you are calling these queries from something sufficiently smart (java, etc) then there should be a way to use string concatenation to create the query string that you actually need.

However, I agree that this is the sort of database design that ends up on the daily wtf. Such tricks might make sense in cases where you are pushing the envelop of what relational databases are good for. However, this sort of "event management system" could be an example in a book as something that relational databases are good for.

User avatar
the pie
Posts: 29
Joined: Tue Mar 17, 2009 7:55 am UTC
Location: Oz-tray-lee-ar, brisbane

Re: The "IT DOESN'T WORK!" thread

Postby the pie » Wed May 18, 2011 7:28 am UTC

Unfortunately no, I'm not calling the queries from java or the like :(

I did suggest that all events be put in a single table e.g.

ID, EventName, Type, Location, Date, General

As in all 'event specific' information would go into the 'general' column.

But unluckily they want to be able to have a reporting mechanism for each event type. So each event type needs to have different columns i.e. the 'carnival' event type is going to have all the columns I listed above as well as some that are specific to just carnivals. Thus, having every event type in one table is no longer feasible.

Their solution to this problem has gotten quite messy though, and is making other people's jobs quite hard.

So now I'm trying to figure out how to search all columns from all tables (even ones that dont exist yet *facepalm*) and I have come across a stored procedure that I may be able to modify, but it seems this is the only solution to the problem T_T

User avatar
phlip
Restorer of Worlds
Posts: 7573
Joined: Sat Sep 23, 2006 3:56 am UTC
Location: Australia
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby phlip » Wed May 18, 2011 7:58 am UTC

Thinking aloud, in pseudo-DDL:

Code: Select all

CREATE TABLE event_type (
  event_type_id primary key,
  description,
  any other fields you want
);
CREATE TABLE event (
  event_id primary key,
  event_type_id foreign key,
  description,
  time,
  etc
);
CREATE TABLE extra_property (
  property_id primary key,
  event_type foreign key,
  description,
  maybe some other fields... "datatype" and "is_mandatory" come to mind
);
CREATE TABLE event_property (
  event_property_id primary key,
  event_id foreign key,
  property_id foreign key,
  value
);

That's the basic idea, anyway. The code to pull the data out of those tables (getting the list of event types from event_type and extra columns from extra_property) isn't going to be any more painful than what you have now (getting the event types from INFORMATION_SCHEMA.TABLES and the extra columns from INFORMATION_SCHEMA.COLUMNS)...

I'm not sure what the usual name for this pattern is... at work, we call it an "extendable table" or a "vertical table" (because the fields of each "record" are spanned over many rows in event_property, as opposed to a standard horizontal table, where they're spanned over the columns of the table).

Code: Select all

enum ಠ_ಠ {°□°╰=1, °Д°╰, ಠ益ಠ╰};
void ┻━┻︵​╰(ಠ_ಠ ⚠) {exit((int)⚠);}
[he/him/his]

User avatar
the pie
Posts: 29
Joined: Tue Mar 17, 2009 7:55 am UTC
Location: Oz-tray-lee-ar, brisbane

Re: The "IT DOESN'T WORK!" thread

Postby the pie » Thu May 19, 2011 4:18 am UTC

Hey thanks, man! That looks much easier to use! Enjoy some good karma :mrgreen:

User avatar
RoadieRich
The Black Hand
Posts: 1037
Joined: Tue Feb 12, 2008 11:40 am UTC
Location: Behind you

Re: The "IT DOESN'T WORK!" thread

Postby RoadieRich » Thu May 19, 2011 11:19 am UTC

phlip wrote:Thinking aloud, in pseudo-DDL:

Code: Select all

CREATE TABLE event_type (
  event_type_id primary key,
  description,
  any other fields you want
);
CREATE TABLE event (
  event_id primary key,
  event_type_id foreign key,
  description,
  time,
  etc
);
CREATE TABLE extra_property (
  property_id primary key,
  event_type foreign key,
  description,
  maybe some other fields... "datatype" and "is_mandatory" come to mind
);
CREATE TABLE event_property (
  event_property_id primary key,
  event_id foreign key,
  property_id foreign key,
  value
);

That's the basic idea, anyway. The code to pull the data out of those tables (getting the list of event types from event_type and extra columns from extra_property) isn't going to be any more painful than what you have now (getting the event types from INFORMATION_SCHEMA.TABLES and the extra columns from INFORMATION_SCHEMA.COLUMNS)...

I'm not sure what the usual name for this pattern is... at work, we call it an "extendable table" or a "vertical table" (because the fields of each "record" are spanned over many rows in event_property, as opposed to a standard horizontal table, where they're spanned over the columns of the table).

Can you set up a constraint (or similar) to ensure that the event_type foreign key on event_property matches the event_type on event_type joined to event? Or would you need to do it in application code?

Does the first sentence above actually make sense?
73, de KE8BSL loc EN26.

User avatar
hotaru
Posts: 1045
Joined: Fri Apr 13, 2007 6:54 pm UTC

Re: The "IT DOESN'T WORK!" thread

Postby hotaru » Fri May 20, 2011 9:39 pm UTC

so, i just tried compiling this code with my normal CFLAGS and it didn't work right.

Code: Select all

#include <stdio.h>

int main(void)
{
 struct num{ unsigned int a:3, b:3, c:2; } n = {0};
  unsigned char *= &n;
  do do printf("%hhu\n", *c);
  while(!(n.a-- && !++n.b));
  while(++n.c);
  return 0; } 


if i compile it without -O, i get the correct results:
Spoiler:
0
7
14
21
28
35
42
49
56
63
70
77
84
91
98
105
112
119
126
133
140
147
154
161
168
175
182
189
196
203
210
217
224
231
238
245
252


if i compile it with -O, i get these wrong results:
Spoiler:
0
7
14
21
28
35
42
49
56
71
78
85
92
99
106
113
120
135
142
149
156
163
170
177
184
199
206
213
220
227
234
241
248


but if i copy the list of all the options that the man page says are turned on by -O, i get the correct results.

any ideas what's going on here?

Code: Select all

factorial product enumFromTo 1
isPrime n 
factorial (1) `mod== 1

User avatar
Dason
Posts: 1311
Joined: Wed Dec 02, 2009 7:06 am UTC
Location: ~/

Re: The "IT DOESN'T WORK!" thread

Postby Dason » Fri May 20, 2011 10:24 pm UTC

I don't get any troubles when I compile it with -O

What are you usual cflags and what version of gcc are you using? Or are you using a different compiler?
double epsilon = -.0000001;

User avatar
hotaru
Posts: 1045
Joined: Fri Apr 13, 2007 6:54 pm UTC

Re: The "IT DOESN'T WORK!" thread

Postby hotaru » Fri May 20, 2011 11:49 pm UTC

Dason wrote:I don't get any troubles when I compile it with -O

What are you usual cflags and what version of gcc are you using? Or are you using a different compiler?

"-O2 -s -Wall -w -pedantic" and gcc 4.6. i just tried gcc 4.5 and it works fine with -O.

Code: Select all

factorial product enumFromTo 1
isPrime n 
factorial (1) `mod== 1

User avatar
phlip
Restorer of Worlds
Posts: 7573
Joined: Sat Sep 23, 2006 3:56 am UTC
Location: Australia
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby phlip » Sat May 21, 2011 7:55 am UTC

The first wrong answer is it going from 56 to 71... which is, unless the optimiser does something weird with the structure packing, {.a=0, .b=7, .c=0} to {.a=7, .b=0, .c=1}.

Which means it's likely doing the n.a--, which should return 0, so the && should short-circuit, but it's doing the ++n.b anyway, which correctly returns 0, so it drops out and does the ++n.c too.

I believe the bits involving the "c" variable are the only not-strictly-defined parts of that code... take those out (say, replace it with printf("%d, %d, %d\n", n.a, n.b, n.c);) and everything should be strictly legal. If you're still getting errors, maybe it's time to pull apart the disassembly and see what it's doing, maybe think about sending a bug report to the gcc guys.

Can you post the gcc -S? I'd tinker myself, but I only have 4.5.2, and it works for me.

Code: Select all

enum ಠ_ಠ {°□°╰=1, °Д°╰, ಠ益ಠ╰};
void ┻━┻︵​╰(ಠ_ಠ ⚠) {exit((int)⚠);}
[he/him/his]

User avatar
TheChewanater
Posts: 1279
Joined: Sat Aug 08, 2009 5:24 am UTC
Location: lol why am I still wearing a Santa suit?

Re: The "IT DOESN'T WORK!" thread

Postby TheChewanater » Sat May 21, 2011 4:33 pm UTC

It works fine for me in GCC 4.6.
ImageImage
http://internetometer.com/give/4279
No one can agree how to count how many types of people there are. You could ask two people and get 10 different answers.

Osha
Posts: 727
Joined: Wed Dec 03, 2008 3:24 am UTC
Location: Boise, Idaho, USA

Re: The "IT DOESN'T WORK!" thread

Postby Osha » Mon May 23, 2011 3:34 am UTC

This is almost certainly something simple, but it's also something hard to google for.

code segfaults on "lookup["foo"] = 32;"
Spoiler:

Code: Select all

//foo.h
struct Bar;

class Foo{
private:
    Bar* kitty;

public:
    Foo();
    ~Foo();
};

Code: Select all

//bar.h
class Bar {
public:
    Bar();
};

Code: Select all

//main.cpp
#include "foo.h"

Foo f;

int main(int argc, char** argv) {
    return 0;
}

Code: Select all

//foo.cpp
#include "foo.h"
#include "bar.h"

Foo::Foo() {
    kitty = new Bar();
}

Foo::~Foo() {
    delete kitty;
}

Code: Select all

//bar.cpp
#include <map>
#include <string>
#include "bar.h"

std::map<std::string,int> lookup;

Bar::Bar() {
    lookup["foo"] = 32;
}


What I'm guessing is happening here is that C++ doesn't guarantee that an object declared there (global scope, will ever be constructed? But why is the Foo in main.cpp being constructed like expected, what's different? The map works if I change it to a pointer and initialize it with null (and initialize it later on of course), but are things like that guaranteed to work (plain old data, constant initializations)? Is there any way to keep this sort of logic without making lookup a static class member or pointer or something?
Hopefully that all made sense @_@. C++ is so confusing sometimes, and always full of weird little quirks! But I'm stuck with it for this project...
Any insight or reading material would be appreciated :3

User avatar
jaap
Posts: 2094
Joined: Fri Jul 06, 2007 7:06 am UTC
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby jaap » Mon May 23, 2011 3:55 am UTC

Osha wrote:This is almost certainly something simple, but it's also something hard to google for.


I googled "c++ initialization global variable order" and got some good answers.
It seems that while the initialisation of global variables in a single file are done in the order they occur in the file, when you look at globals in different files there are no guarantees and there is no checking for dependencies.

In your case, foo is declared in main but to instantiate it it already needs lookup to exist ( because it instantiates a Bar) and this is not guaranteed.

User avatar
phlip
Restorer of Worlds
Posts: 7573
Joined: Sat Sep 23, 2006 3:56 am UTC
Location: Australia
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby phlip » Mon May 23, 2011 3:59 am UTC

Well, your code has two global objects - a Foo called f, and a map called lookup. The language makes no guarantees about which one will be initialised first... they're just both globals, in different source files.

The C++ FAQ has some suggestions on how to do it... which basically boils down to "make lookup not a global, construct it yourself". Instead of having a map<etc> lookup, have a map<etc> *lookup, and create a new map<etc> the first time you need it. That way you can control the order that everything gets constructed, and make sure it gets constructed before it gets used.

Code: Select all

enum ಠ_ಠ {°□°╰=1, °Д°╰, ಠ益ಠ╰};
void ┻━┻︵​╰(ಠ_ಠ ⚠) {exit((int)⚠);}
[he/him/his]

Osha
Posts: 727
Joined: Wed Dec 03, 2008 3:24 am UTC
Location: Boise, Idaho, USA

Re: The "IT DOESN'T WORK!" thread

Postby Osha » Mon May 23, 2011 4:38 am UTC

Ah thanks! That makes a lot of sense.

User avatar
hotaru
Posts: 1045
Joined: Fri Apr 13, 2007 6:54 pm UTC

Re: The "IT DOESN'T WORK!" thread

Postby hotaru » Mon May 23, 2011 1:29 pm UTC

phlip wrote:The first wrong answer is it going from 56 to 71... which is, unless the optimiser does something weird with the structure packing, {.a=0, .b=7, .c=0} to {.a=7, .b=0, .c=1}.

Which means it's likely doing the n.a--, which should return 0, so the && should short-circuit, but it's doing the ++n.b anyway, which correctly returns 0, so it drops out and does the ++n.c too.

I believe the bits involving the "c" variable are the only not-strictly-defined parts of that code... take those out (say, replace it with printf("%d, %d, %d\n", n.a, n.b, n.c);) and everything should be strictly legal. If you're still getting errors, maybe it's time to pull apart the disassembly and see what it's doing, maybe think about sending a bug report to the gcc guys.

changing it to printf("%d, %d, %d\n", n.a, n.b, n.c); gives correct results either way... but if i add that after the printf("%hhu\n", *c); (with braces to keep both lines in the loop), i get this with -O:

Code: Select all

56
56, 7, 0
71
71, 0, 1
78
78, 1, 1


so apparently the printf("%hhu\n", *c) is changing the size of .a. my gcc 4.6 is not the most up to date, so it's possible that it's a bug that's already been discovered and fixed... i'll update and see if it still happens.

Code: Select all

factorial product enumFromTo 1
isPrime n 
factorial (1) `mod== 1

User avatar
phlip
Restorer of Worlds
Posts: 7573
Joined: Sat Sep 23, 2006 3:56 am UTC
Location: Australia
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby phlip » Mon May 23, 2011 1:41 pm UTC

Hmm. If I had to guess based on that (and I'd want to check the disassembly to make sure, before blaming this definitively) it seems like it's rolled the "n.a" and "*c" parts together... it thinks they're reading from the same place (which they are), so it's doing common expression elimination and collapsing the two reads together, even though they're actually different. And as a result, it's forgetting to do the bitmask for n.a.

Code: Select all

enum ಠ_ಠ {°□°╰=1, °Д°╰, ಠ益ಠ╰};
void ┻━┻︵​╰(ಠ_ಠ ⚠) {exit((int)⚠);}
[he/him/his]

Markus__1
Posts: 36
Joined: Wed Apr 20, 2011 6:42 pm UTC

Re: The "IT DOESN'T WORK!" thread

Postby Markus__1 » Sun Jun 12, 2011 7:34 pm UTC

hotaru wrote:so, i just tried compiling this code with my normal CFLAGS and it didn't work right.

Code: Select all

#include <stdio.h>

int main(void)
{
 struct num{ unsigned int a:3, b:3, c:2; } n = {0};
  unsigned char *= &n;
  do do printf("%hhu\n", *c);
  while(!(n.a-- && !++n.b));
  while(++n.c);
  return 0; } 


<slightly paraphrased>
if i compile it without -O, i get correct results.
if i compile it with -O, i get wrong results.
but if i copy the list of all the options that the man page says are turned on by -O, i get correct results.
any ideas what's going on here?


Well, let's see

Code: Select all

$ gcc -Wall -Wextra orig_bitfield.c   # gcc 4.3.4 cygwin
orig_bitfield.c: In function `main':
orig_bitfield.c:4: warning: missing initializer
orig_bitfield.c:4: warning: (near initialization for `n.b')
orig_bitfield.c:5: warning: initialization from incompatible pointer type


Problems diagnosed
  • You're only initalizing n.a, you should write n = { 0, 0, 0 }
  • You're taking a char pointer to an int, expecting to hit those 8 bits that get assigned to your bitfield. This only works because your GCC allocates from the lower bits and because you are on a little-endian machine (it would also work for big-endian and allocation from the high bits). The former is defined by the ABI, the latter by your hardware (and some options on bi-endian machines :-) ) See the gcc docs - http://gcc.gnu.org/onlinedocs/gcc/Structures-unions-enumerations-and-bit_002dfields-implementation.html

So, questions:
  • Does it help to initalize all fields?
  • If not, does the problem also happen for a struct of 3 unsigned chars, using modulo or masking for wraparound?

If initializing all 3 fields does not help, you should probably take your question to the gcc-help mailing list.

User avatar
phlip
Restorer of Worlds
Posts: 7573
Joined: Sat Sep 23, 2006 3:56 am UTC
Location: Australia
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby phlip » Mon Jun 13, 2011 1:23 am UTC

Markus__1 wrote:You're only initalizing n.a, you should write n = { 0, 0, 0 }

Nope:
ISO/IEC 9899:1999 (E) §6.7.8¶21 wrote:If there are fewer initializers in a brace-enclosed list than there are elements or members of an aggregate, or fewer characters in a string literal used to initialize an array of known size than there are elements in the array, the remainder of the aggregate shall be initialized implicitly the same as objects that have static storage duration.

By which they're referring to:
ISO/IEC 9899:1999 (E) §6.7.8¶10 wrote:If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. If an object that has static storage duration is not initialized explicitly, then:
  • if it has pointer type, it is initialized to a null pointer;
  • if it has arithmetic type, it is initialized to (positive or unsigned) zero;
  • if it is an aggregate, every member is initialized (recursively) according to these rules;
  • if it is a union, the first named member is initialized (recursively) according to these rules.

Or, in short: if you only specify an incomplete initialiser, the rest get initialised with 0's.

Code: Select all

enum ಠ_ಠ {°□°╰=1, °Д°╰, ಠ益ಠ╰};
void ┻━┻︵​╰(ಠ_ಠ ⚠) {exit((int)⚠);}
[he/him/his]

Markus__1
Posts: 36
Joined: Wed Apr 20, 2011 6:42 pm UTC

Re: The "IT DOESN'T WORK!" thread

Postby Markus__1 » Mon Jun 13, 2011 9:39 am UTC

phlip wrote:
Markus__1 wrote:You're only initalizing n.a, you should write n = { 0, 0, 0 }

Nope:
<snip the cited standard>
Or, in short: if you only specify an incomplete initialiser, the rest get initialised with 0's.


OK, then it looks like a case for gcc-help.

User avatar
TheChewanater
Posts: 1279
Joined: Sat Aug 08, 2009 5:24 am UTC
Location: lol why am I still wearing a Santa suit?

Re: The "IT DOESN'T WORK!" thread

Postby TheChewanater » Tue Jun 28, 2011 11:53 pm UTC

I have a Vector class:

Code: Select all

template<class T> struct Vector
{
  T x, y, z, w;
  
  
// Returns "[x,y,z,w]".  I know this works correctly.
  operator char const* ();
  
  
// Some other methods...
}; 


And a matrix class,

Code: Select all

template<class T> struct Matrix
{
  T t[4][4];
  
  Vector
<T> row (char r);
  
  
// Some other methods...
}; 


Here's the definition of Matrix::row (except with bounds checking and const-correctness and stuff). As far as I can tell, it doesn't invoke undefined behavior - structs and arrays have to be contiguous memory, right?

Code: Select all

Vector<T> row (char row)
{
  return *(Vector<T>*)(&(t[row]));

  /* Just in case they don't, I tried it with the following, too.
    I'm not sure if this is less efficient (this could be called thousands of times per second)*/
  return Vector<T> (t[row][0], t[row][1], t[row][2], t[row][3]);
}
 


All of this seems to work in any test case I run, except the following.

Code: Select all

Matrix<float> m (
  0,  1,  2,  3,
  4,  5,  6,  7,
  8,  9,  10, 11,
  12, 13, 14, 15);

cout << m.row (0) << '\n';
cout << m.row (1) << '\n';
// This prints "[0,1,2,3]" and "[4,5,6,7]".  Okay, cool.

cout << m.row (0) << '\n' << m.row (1) << '\n'
// This prints "[0,1,2,3]" and "[0,1,2,3]".  Err... what?

ostream& out = (std::cout << m.row (0) << '\n');
out << m.row (1) << '\n';
/* This seems like it should be just like the second one, but it prints
  "[0,1,2,3]" and "[4,5,6,7]".  Okay, now C++ is just shitting with me.*/
 
ImageImage
http://internetometer.com/give/4279
No one can agree how to count how many types of people there are. You could ask two people and get 10 different answers.

User avatar
phlip
Restorer of Worlds
Posts: 7573
Joined: Sat Sep 23, 2006 3:56 am UTC
Location: Australia
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby phlip » Wed Jun 29, 2011 1:25 am UTC

OK, first off: that cast is scary, and it is undefined behavior (for one, structs aren't required to be contiguous, there can be padding between the members... also, even if the representations of your struct and an array are identical, the spec doesn't say you can cast like that from one to the other). However, with most sane compilers it will work, as long as you avoid the padding thing (eg with #pragma pack or equivalent if you're using a small type for T).

I think the error is probably in your Vector->cstring conversion, which probably reuses memory, or returns a pointer to the stack, or something. Try this:

Code: Select all

char const *a = m.row(0);
std::out << a << std::endl;
char const *b = m.row(1);
std::out << a << std::endl;

If my suspicions are right, this will output [0,1,2,3] the first time, but [4,5,6,7] the second time, or something else that's unexpected, even though it's coming from the same variable both times.

Code: Select all

enum ಠ_ಠ {°□°╰=1, °Д°╰, ಠ益ಠ╰};
void ┻━┻︵​╰(ಠ_ಠ ⚠) {exit((int)⚠);}
[he/him/his]

EvanED
Posts: 4331
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby EvanED » Wed Jun 29, 2011 1:57 am UTC

That's my suspicion too. If you were asking me in person, I would wager $50 that your code is doing one of the following: reusing a static buffer, returning stack space, leaking memory, or something worse. The first two would easily explain the behavior you're seeing. I'm pretty sure the only way you can make that (pretty horrid, in all honesty) conversion operator work is to have multiple static buffers and cycle between them: which is just delaying the problem until you have n+1 calls to operator const char in one statement.

The critical change between your second-to-last and last example is that you're introducing a sequence point, which forces the compiler to do the output in the right order with respect to the conversion operators.

User avatar
TheChewanater
Posts: 1279
Joined: Sat Aug 08, 2009 5:24 am UTC
Location: lol why am I still wearing a Santa suit?

Re: The "IT DOESN'T WORK!" thread

Postby TheChewanater » Wed Jun 29, 2011 2:34 am UTC

phlip wrote:OK, first off: that cast is scary, and it is undefined behavior (for one, structs aren't required to be contiguous, there can be padding between the members... also, even if the representations of your struct and an array are identical, the spec doesn't say you can cast like that from one to the other). However, with most sane compilers it will work, as long as you avoid the padding thing (eg with #pragma pack or equivalent if you're using a small type for T).

Yeah, I changed that cast to something more sane. I'm pretty sure there's no performance hit, since everything is copied anyways when it's dereferenced and returned.

Here's how I convert Vectors to strings.

Code: Select all

inline operator char const* () const
{
    std::stringstream ss;
    ss << '[' << x << ',' << y << ',' << z << ',' << w << ']';

    return ss.str ().c_str ();
}
 


The following seems to work, if I assume T is float.

Code: Select all

inline operator char const* () const
{
    char* str = new char[64];
    sprintf (str, "[%f,%f,%f,%f]", x, y, z, w);
    
    return str
;
}
 


So, is there anyway to get it to work with the type-safe stringstream?
ImageImage
http://internetometer.com/give/4279
No one can agree how to count how many types of people there are. You could ask two people and get 10 different answers.

EvanED
Posts: 4331
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby EvanED » Wed Jun 29, 2011 2:56 am UTC

TheChewanater wrote:Here's how I convert Vectors to strings.

Code: Select all

inline operator char const* () const
{
    std::stringstream ss;
    ss << '[' << x << ',' << y << ',' << z << ',' << w << ']';

    return ss.str ().c_str ();
}
 

Yep, that's the problem. For starters, the return of c_str() isn't guaranteed to live beyond the end of the full expression containing it. (I think that's the lifetime.) In other words, even the following is technically not kosher:

Code: Select all

std::stringstream ss;
ss << ...;
const char * c = ss.str().c_str();
cout << c << "\n";

all in one procedure. Realistically, I suspect that you can reliably expect the return value from c_str() to last until either the corresponding string is destructed or it changes in a way that may require reallocation (e.g. appending), though don't. But you're not even doing that: you're relying on the return value from c_str() to live beyond the lifetime of the containing string object (contained inside the stringstream).

The following seems to work, if I assume T is float.

Code: Select all

inline operator char const* () const
{
    char* str = new char[64];
    sprintf (str, "[%f,%f,%f,%f]", x, y, z, w);
    
    return str
;
}
 


So, is there anyway to get it to work with the type-safe stringstream?

Don't do this. It's horrid, truly horrid. I wasn't kidding above.

If you write

Code: Select all

cout << myvec;
with that code and you're leaking memory.

Write an operator string instead, and return ss.str(). Or even better, overload operator<< for your class, if this is the reason you want that operator in the first place.

If you really insist on using a const char* return value, make it a function that you have to call explicitly, and/or return a smart pointer (perhaps an auto_ptr<const char>). You can still use stringstream to construct the result, then strdup it (or otherwise manually make a copy).

But if you want a typesafe way to construct a const char* (edit: without that strdup) that you can return from your function, there isn't one, at least not in the standard.

You should be quite suspicious of implicit conversion operators and somewhat suspicious of functions that return pointers to memory that they allocate. Combining the two is a recipe for disaster; if I were the maintainer of some code base and you submitted a patch with that in it, there's no way in hell I'd accept it without a damn good reason.

User avatar
TheChewanater
Posts: 1279
Joined: Sat Aug 08, 2009 5:24 am UTC
Location: lol why am I still wearing a Santa suit?

Re: The "IT DOESN'T WORK!" thread

Postby TheChewanater » Wed Jun 29, 2011 3:05 am UTC

EvanED wrote:Write an operator string instead, and return ss.str(). Or even better, overload operator<< for your class, if this is the reason you want that operator in the first place

Yeah, a string operator seems like a much better idea anyway. It does require explicit casting, so I'll probably define an operator<<.
ImageImage
http://internetometer.com/give/4279
No one can agree how to count how many types of people there are. You could ask two people and get 10 different answers.

User avatar
Ptolom
Posts: 1559
Joined: Mon Mar 24, 2008 1:55 pm UTC
Location: The entropy pool
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby Ptolom » Thu Jun 30, 2011 5:47 pm UTC

I have a question
I have an abstract class like this

Code: Select all

class parent{
  protected:
    int numbers[10];
....
}

and then a load of child classes in which I need to hard code the values of numbers. Is there a way of using an array initialiser list in each child? I could use pointers but it seems messy.

EvanED
Posts: 4331
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby EvanED » Thu Jun 30, 2011 6:39 pm UTC

I'm pretty sure no (though I'm not sure what support you'd get from C++0x if you wanted to use that).

But you can do the following:

Code: Select all

class child : public parent {
    child() {
        int init[] = {1,2,3,4,5,6,7,8,9,10};
        std::copy(init, init+10, this->numbers);
    }
    ...
};


That's pretty good, except for the lack of bounds-checking.

User avatar
TheChewanater
Posts: 1279
Joined: Sat Aug 08, 2009 5:24 am UTC
Location: lol why am I still wearing a Santa suit?

Re: The "IT DOESN'T WORK!" thread

Postby TheChewanater » Thu Jun 30, 2011 7:34 pm UTC

You could use std::vector<int>, which can be resized.
ImageImage
http://internetometer.com/give/4279
No one can agree how to count how many types of people there are. You could ask two people and get 10 different answers.

User avatar
Yakk
Poster with most posts but no title.
Posts: 11129
Joined: Sat Jan 27, 2007 7:27 pm UTC
Location: E pur si muove

Re: The "IT DOESN'T WORK!" thread

Postby Yakk » Mon Jul 04, 2011 12:53 pm UTC

EvanED wrote:I'm pretty sure no (though I'm not sure what support you'd get from C++0x if you wanted to use that).

But you can do the following:

Code: Select all

class child : public parent {
    child() {
        int init[] = {1,2,3,4,5,6,7,8,9,10};
        std::copy(init, init+10, this->numbers);
    }
    ...
};


That's pretty good, except for the lack of bounds-checking.

Code: Select all

class parent{
  protected:
    int numbers[10];
    template<size_t n>
    void init_numbers( int numbers_[n] )
    {
        // TODO: static_assert n == 10 if your compiler supports that C++0x feature
        Assert(n==10);
        size_t cnt = n>10:10:n;
        std::copy(numbers_, numbers_+cnt, this->numbers);
....
};

class child : public parent {
    child() {
        int init[] = {1,2,3,4,5,6,7,8,9,10};
        init_numbers(init);
    }
    ...
};

Either the above, or a slight syntactic twerk of the above, also does bounds-checking.

I could in non-C++-0x do a pseudo-static assert by making init_numbers not accept any array that isn't size 10, but the syntax is obscure and usually not worth it.

Code: Select all

template<size_t n>
struct only_ten_allowed {};

template<>
struct only_ten_allowed<10>{typedef void result;}

class parent{
  protected:
    int numbers[10];
    template<size_t n>
    only_ten_allowed<n>::result init_numbers( int numbers_[n] )
    {
        std::copy(numbers_, numbers_+n, this->numbers);
....
};

... but readability is more important than being provably correct usually, and few people can read the above and understand how/why it works, so I wouldn't use that technique.
One of the painful things about our time is that those who feel certainty are stupid, and those with any imagination and understanding are filled with doubt and indecision - BR

Last edited by JHVH on Fri Oct 23, 4004 BCE 6:17 pm, edited 6 times in total.

EvanED
Posts: 4331
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby EvanED » Mon Jul 04, 2011 5:23 pm UTC

You can get a static assert in C++98/03 too; it's really not that hard. You've essentially done it, except you've specialized the case for making sure the array is size 10. (I do think it's worth the extra syntax if you wrap it up like Boost does.)

Anyway, I'm not even sure you can do that. I tried the following code under GCC 4.3.2, 4.6.1, and Comeau and none accepted it: all complained about no matching function for the call:
Spoiler:

Code: Select all

#include <algorithm>

class parent{
  protected:
    int numbers[10];
    template<size_t n>
    void init_numbers( int numbers_[n] )
    {
        size_t cnt = n>10?10:n;
        std::copy(numbers_, numbers_+cnt, this->numbers);
    }
};

class child : public parent {
    child() {
        int init[] = {1,2,3,4,5,6,7,8,9,10};
        init_numbers(init);
    }
};


If there's some easy change I'm missing, there's still one more thing I'd recommend, which is to do this:

Code: Select all

#define NUM_ELEMENTS(array) (sizeof(array)/sizeof((array)[0]))

class parent {
  protected:
    int numbers[10];
    const int numbers_size;

    parent()
        : numbers_size(NUM_ELEMENTS(numbers))
    {}

and then use numbers_size. I like this even more than using the same constant for both the array size and when doing checking; it just removes the possibility for one more mistake.

User avatar
Yakk
Poster with most posts but no title.
Posts: 11129
Joined: Sat Jan 27, 2007 7:27 pm UTC
Location: E pur si muove

Re: The "IT DOESN'T WORK!" thread

Postby Yakk » Tue Jul 05, 2011 6:07 pm UTC

Sorry: you need a reference to the array, not the array itself, for it to work.

I just wasn't confident on the syntax, so I did a bare array, which (apparently) doesn't match properly.

I think it is:

Code: Select all

template<size_t n>
void func( int (&array)[n] )
{
}

mayhap.
One of the painful things about our time is that those who feel certainty are stupid, and those with any imagination and understanding are filled with doubt and indecision - BR

Last edited by JHVH on Fri Oct 23, 4004 BCE 6:17 pm, edited 6 times in total.

User avatar
Lorenz
Posts: 27
Joined: Fri Apr 29, 2011 5:57 am UTC

Re: The "IT DOESN'T WORK!" thread

Postby Lorenz » Thu Jul 07, 2011 12:34 am UTC

I wrote a simple, small program in C, for a silly thing I wanted to test.
The results I wanted to print, were simply not printing, so I inserted a print statement at the beginning of the for loop to see if it was even entering the loop.
The results came back out just the way I wanted them, with the many printed test statements behind them... So I deleted the print statement.
It was now not working.

I had to stick a
printf(" \b"); (Single space before the \b, added 2 for emphasis)
at the start of the loop. It works perfectly, but I can not figure out why.... Stupid atoms.

User avatar
thoughtfully
Posts: 2253
Joined: Thu Nov 01, 2007 12:25 am UTC
Location: Minneapolis, MN
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby thoughtfully » Thu Jul 07, 2011 12:59 am UTC

Could be a missing header file. Are you compiling with -Wall?
Image
Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away.
-- Antoine de Saint-Exupery

User avatar
Lorenz
Posts: 27
Joined: Fri Apr 29, 2011 5:57 am UTC

Re: The "IT DOESN'T WORK!" thread

Postby Lorenz » Thu Jul 07, 2011 1:52 am UTC

I've never compiled with -Wall. I just did, and it prints out only the first loop of the for, and then stops. If I add the print, it still prints all of it.
(What does -Wall do?)

Code: Select all

int *b10tobn(int num10, int base);
int ispali(int *num);

int main(int argc, char *argv[])
{
  int i , *aux , num , base ;
  num = atoi(argv[1]);
  for(base = 2; base < num; base++)
    {
      printf(" %c",8);
      aux = b10tobn(num,base);
      if(ispali(aux))
   {
     printf("Base%d = ",base);
     for(i = 1 ; i<=aux[0] ; i++)
       {
         printf("'%d'",aux[i]);
       }
     printf("\n");
   }
    }
  return 0;
}

int *b10tobn(int num10, int base)
{
  int length , aux, *numb;
  aux = num10;
  while(aux != 0)
    {
      aux = aux / base;
      length++;
    }
  numb = (int*) malloc (length + 1);
  numb[0] = length;
  aux = num10;
  while(length > 0)
    {
      numb[length] = aux % base;
      aux = aux/base;
      length--;
    }
  return numb;
}


int ispali(int *num)
{
  int length , i;
  length = num[0];
  for(i = 1 ; i <= length - i + 1 ; i++)
    {
      if(num[i]!=num[length-i+1])
   return 0;
    }
  return 1;
}

User avatar
phlip
Restorer of Worlds
Posts: 7573
Joined: Sat Sep 23, 2006 3:56 am UTC
Location: Australia
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby phlip » Thu Jul 07, 2011 10:25 am UTC

Lorenz wrote:(What does -Wall do?)

It turns on all the commonly-used compiler warnings. It shouldn't change the actual program itself - just the messages the compiler gives you while it compiles.

The problem you're having (change a line of code and completely unrelated changes, are usually down to a few things:
* Dangling pointers - that is, trying to refer to memory that isn't allocated any more. Eg trying to read from a pointer after it's been freed, or making a pointer to a variable on the stack, and trying to use it after that function has returned, or the variable has gone out of scope. This is the #1 single most worstest ever thing to debug, so be careful when you're playing with pointers, so you can avoid it.
* Uninitialised variables - these contain random data - just whatever happened to be there at the time. This includes both local variables that don't have an initialiser, and the memory returned by malloc(). Always initialise or set variables before reading from them.
* Floating point precision - floating-point numbers can change in the very very small scale, depending on the surrounding code... 99.99% of the time this isn't a problem, and not worth worrying about, and the difference will be lost in the normal rounding errors. I'm only including this here for completeness.

In this case, your problem is number 2. Look at your b10tobn function. None of your locals are initialised. aux and numb are assigned before they're used, but length is not. So, if you come in, and length happens to be storing the number 15835, and it calculates that it needs to make a 10-digit-long array... it's going to instead start counting from 15835 and try to create a 15845-digit-long array. Most likely you will soon run out of memory, as it's going to be trying to allocate on the order of gigabytes of memory on every loop. In my case, this means it fails outright... on your machine, it sounds like it's compiling and running, but then it takes a long time to actually calculate a billion digits of a base-n number, so it's taking a long time to run - it looks like it's hanged.
As for why adding the printf helps? Well, that's what makes these sorts of bugs so painful to track down - all the little workings all get tangled up. Most likely something internal in printf happened to be allocated in the same place that length was put later... and that internal thing in printf happened to be 0, or something very small, when printf finished. So when b10tobn is called, length just happens to start at 0, or close to it. But that's only a guess - there's no way to know for sure.

Other, slightly less pressing points:
* You have a malloc(), but not a free(). This is going to leak memory.
* You don't have any headers. For this, you should at least be importing <stdio.h> (for printf) and <stdlib.h> (for atoi, malloc and free). That your code is compiling without the headers is just luck, and the compiler warnings should tell you about this if you have them turned on.
* You probably should have some kind of error message if argc<2. Otherwise your program will just crash. Failing if the thing they enter isn't a number would be nice too, but a little trickier, especially since you're using atoi(), which doesn't check for errors.
* b10tobn is misnamed - it's not converting base-10 to base-n, it's convering an int to base-n. Don't try to think of an int as a base-10 number... they're separate things. It's just a number when you are doing maths stuff to it, and it's a base-10 number when you have it as a string. Your original input to the program is a base-10 number from the command line, in argv[1], but then you use atoi to convert it from base-10 to an int, which you can then do maths with.

Code: Select all

enum ಠ_ಠ {°□°╰=1, °Д°╰, ಠ益ಠ╰};
void ┻━┻︵​╰(ಠ_ಠ ⚠) {exit((int)⚠);}
[he/him/his]

User avatar
Lorenz
Posts: 27
Joined: Fri Apr 29, 2011 5:57 am UTC

Re: The "IT DOESN'T WORK!" thread

Postby Lorenz » Thu Jul 07, 2011 3:40 pm UTC

None of your locals are initialised. aux and numb are assigned before they're used, but length is not

Facepalm... Thanks! I usually figure those errors out with unexpected behavior, not expected strange behavior.

* You have a malloc(), but not a free(). This is going to leak memory.
Fix-d

* You don't have any headers
I do... sorry for not copying them.

* You probably should have some kind of error message, * b10tobn is misnamed.
It's for my own personal use only, so I didn't worry about those things, but thank you!

EvanED
Posts: 4331
Joined: Mon Aug 07, 2006 6:28 am UTC
Location: Madison, WI
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby EvanED » Wed Jul 20, 2011 12:53 am UTC

I have a program that runs differently at the command prompt than in GDB. No multithreading.

My best guess is that it's picking up the wrong GCC runtime, but I don't know if that's true and I don't know how to check it. I also don't know why it would successfully load from within GDB when it fails to load (with version `GLIBCXX_3.4.10' not found) from the shell. I need to set LD_LIBRARY_PATH to get it to run normally, and as part of the GNU project's institution-wide policy to throw in some really awful decisions along with what they do well, GDB ignores LD_LIBRARY_PATH. Still, I set solib-search-path, so I think it should be right... (I also set solib-absolute-prefix to a non-existent directory (!) like the docs say, and also tried set environment LD_LIBRARY_PATH ...)

Edit: whoa, never mind.

Rysto
Posts: 1460
Joined: Wed Mar 21, 2007 4:07 am UTC

Re: The "IT DOESN'T WORK!" thread

Postby Rysto » Wed Jul 27, 2011 10:16 pm UTC

git bisect is an absolutely wonderful thing. I started this afternoon knowing that a 5 year old version of some software worked, but the latest version did not. Some 22000 revisions had been made in between. About 2 hours later, I had pinpointed the exact commit that broke things.

User avatar
woddfellow2
Posts: 20
Joined: Mon Feb 08, 2010 2:41 am UTC
Contact:

Re: The "IT DOESN'T WORK!" thread

Postby woddfellow2 » Sat Jul 30, 2011 12:54 am UTC

Hello, I am trying to learn Python, and I am writing the following to do so.

How do I pass an argument to a function (e.g., --youngest 42)? Here is what I have thus far:

Code: Select all

#!/usr/bin/env python
# Dating Age Range Calculator
# Version 0.3
# by woddfellow2 | http://wlair.us.to/
# Copyright 2009--2011 woddfellow2
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.
#

import sys; # To output to stderr

# Arguments
# Hopefully someday this will allow the user to specify the age as a command
# line option, e.g.:
# $ age --youngest 42
import argparse;

parser = argparse.ArgumentParser(prog='age', description='Calculates the dateable age range for the specified age');
parser.add_argument('--version', action='version', version='Dating Age Range Calculator 0.3');
parser.add_argument('--youngest');
parser.add_argument('--oldest');

args = parser.parse_args();
age = args.youngest;
age = args.oldest;

# Interactive
try:
    print("Dating Age Range Calculator\nVersion 0.3\n");
    age = int(input("How old are you?\n> "));
except ValueError:
    print("ER001: Input must be an integer", file=sys.stderr);
    exit();
except KeyboardInterrupt:
    print("ER002: Caught interrupt signal; exiting...", file=sys.stderr);
    exit();
except EOFError:
    print("ER003: Caught EOF, exiting...", file=sys.stderr);
    exit();

# Calculate age range
def calc_young(age=age):
    young = age / 2 + 7;
    return young;
def calc_old(age=age):
    old = (age - 7) * 2;
    return old;

# Interactive cont.
print("Youngest:", calc_young(), "\nOldest:", calc_old());

The --youngest and --oldest options do not work, instead doing the interactive thing.
1-Crawl 2-Cnfg 3-ATF 4-Exit ?

User avatar
dumbzebra
Posts: 275
Joined: Thu Dec 10, 2009 4:59 pm UTC
Location: Somewhere on the moon.

Re: The "IT DOESN'T WORK!" thread

Postby dumbzebra » Sun Jul 31, 2011 12:28 am UTC

So, I'm just going to join the python nooby help call here:

I have a dictionary that sorts a string (a name) to a tupel of two variable: A "Cost" and a "Value".
I have a function that takes two different tupels and compares them (in value), and returns a bool (whether the first one is bigger than the second one).

From this I'm supposed to build a greedy algorithm that finds the biggest value (which and how many names to choose from the dictionary) to a given limit of cost.
So first I have to find the biggest "Value" in the dictionary. So far so good.

But how am going to make that "comparing function" move up the dictionary, or even start to compare two tupels if a dictionary is unsorted?
And if I only take the values with dictionary.values(), I then don't know what string belonged to the tupel with the biggest value.
Any Ideas?
As the great philosopher Socrates once said: "No."


Return to “Coding”

Who is online

Users browsing this forum: No registered users and 9 guests