Preprocessor seams and assignment of responsibility 2
cslim/include/CSlim/SlimListSerializer.h:cslim/src/CSlim/SlimListSerializer.c:void SlimList_Release(char *serializedResults);void SlimList_Release(char *serializedResults) { if(serializedResults) free(serializedResults); }
Background
In FitNesse, Query Table Results are a list of list of list:- The outer-most list represents “rows”. It collects all objects found. It has zero or more entries.
- The inner-most list represents a single field. It is a key value pair. It is a list of size 2. The first entry is the name of the field (column to FitNesse). The second field is the value. Both are strings.
- The middle list represents a single object. It is a collection of fields. It has zero or more entires.
I understand the need for this representation and it takes a little bit to get it built correctly. So much so, I built a simple tool to do it in java.
C++ is no different. In fact, the authors of cslim though the same thing and they created a C Abstract Data Type to help out (and an add-on method to create the correct final form):
- SlimList
- SlimList_Serializer
They use C because it can be used by both C and C++. I’m using C++ and I wanted to make building query results even easier, so I built a QueryResultAccumulator. The most recent source is in the previous blog and I’ll be putting it on github after I’ve had some more time to work on it.
Here’s the progression to my QueryResultAccumulator class:
- Wrote a Query-Table based fixture and followed the example provided with cslim (thank you for that!)
- Moved the code from functions into methods on a class
- Extracted a class, called SlimListWrapper, which made the fixture code easier to follow.
- Went to get takeout and realized that I had named the class incorrectly and that it was really accumulating query results (thus the name). The SlimList was a mechanism, not an intent.
- Refactored the class into QueryResultAccumulator (I left the original alone, created a new class, copied code from one to the other and changed it around a bit.
Now it might sound like I didn’t have any tests. In fact, I did. I had my original Acceptance Test in FitNesse. I kept running that, so in a sense I was practicing ATDD.
I was not happy with that, because I was not sure that I had properly handled memory allocation correctly. In fact, I had not. The final result is dynamically allocated and I was not releasing that. So I “fixed” it. (It needs to be released after the return from slim back to FitNesse, so the typical pattern is to lazily delete it, or release it in a “destroy” method called after the execution of a single table.)
I have a memory leak?!
I am simplifying this story a bit. So I’m skipping some intermediate results. Ultimately I wrote the following test to check that you could use a single query result accumulator for multiple results correctly:TEST(QueryResultAccumulator, CanProduceFinalResultsMultipleTimes) {
QueryResultAccumulator accumulator;
accumulator.produceFinalResults();
accumulator.produceFinalResults();
}This caused a memory leak of 60 bytes. It was at this point I was up too late and banging my head against a wall. About 3 hours later I figured that out and went to bed. I fixed the problem and sent a patch to authors of cslim in maybe 45 minutes. So I should have gotten more sleep.
Where is that damn thing
In any case, I visually checked the code. I debugged it. I did everything I could initially think of, and I was convinced that my code was correct. (As we’ll see it both was and was not due to a preprocessor seam in cslim.) I got to the point where I even tried different versions of gcc. (I found a parsing error in g++ 4.5 when handling templates, so in desperation and late at night I wasted 5 minutes switching my compiler.) I had the following code in my class: if(result)
free(result);This was the correct thing, but it was in the wrong location. Again, this was related to a preprocessor seam in cslim.
Eureka!
I looked at the cslim code and confirmed it was doing basic C stuff, nothing surprising. It was at that point that I remembered something important:cslim depends on CppUTest and uses a different malloc/free
Ah ha! That’s it. So I tired to recompile my C++ code to use the same thing. However, I was not able to do that. CppUTest’s memory tracking implementation does not work with many of the C++ standard classes like <string> and <vector>. So I could not compile my code using the same approach.
I’m glad this happened becuase it made me realize that it was the wrong place anyway. Here’s the logic:
- CSlim has a preprocessor seam, comile with or without -Dfree=cpputest_free, -Dmalloc=cpputest_malloc
- I’m using a class in cslim to do the allocation, where the policy of allocation is stored
- I should not release the memory but instead allow the cslim library to release the memory becaue it has the allocation policy, and therefore the release policy.
cslim/include/CSlim/SlimListSerializer.h:cslim/src/CSlim/SlimListSerializer.c:void SlimList_Release(char *serializedResults);void SlimList_Release(char *serializedResults) { if(serializedResults) free(serializedResults); }
I updated my QueryResultAccumulator to use SlimList_Release and my false positive disappeared.
It also turns out that this improved symmetry in the library. To allocate and release entries in an SlimList you use the following functions:
- SlimList* SlimList_Create()
- void SlimList_Destroy(SlimList*);
Now to serlalize a list and release the memory later you use:
- SlimList_Serialize
- SlimList_Release
As I write this, I think there’s a better name. I’ll let the authors give it a better name (like SlimList_Release_Serialization_Results). But in any case, if you use a function in the cslim library that allocates something, you use another method in the cslim library to release it.
Since the libray has a preprocessor seam, that symmetry removes a false-positive memory leak.
What took so long?
I had an interesting time with this. Originally I had not released that memory in the class but rather in the unit test. I was working too late and not thinking clearly. I realized that my class needed to manage that.When I called free in the unit test, it was calling the correct version of free, cpputest_free, becasue it was a unit test using CppUTest. When I moved the code into the class, which has no knowledge of CppUTest (nor can it), the flow of the code was correct, but the compilation (preprocessor symbols) were different and it caused a false positive.
Since I changed the code, I assumed it was a problem with how I changed the code. To me more clear, I though it was a code-flow problem, not a preprocessor seam problem. So I spent a lot of time verifyig my code. Once I determined it was correct, I then moved into debugger mode.
It was not long after that when I finally figured out what was going on.
Conslusions
As with many things in life, this is intuitive once you understand it!-)That cslim depends on CppUTest might be questionable. However, if I treat cslim as a (mostly) black box, and I don’t know its allocation policy, I should not assume a deallocation policy.
By putting the responsiblity in the correct library level, it fixed the problem and added symmetry to the overall solution.
I also really enjoyed this (after it was done). I’ve come across memory leaks using CppUTest in the past. Often they were my fault. Sometimes they were not. This was interesting because it both was and was not my fault. I originally had written the code incorrectly. When I put he correct steps in my code, I still had wrong code because I put the responsibility in the wrong place. It really was only correct after I moved the actual implementation into the library and then called it from my code that I had finally written it correctly.
Some C++ Fixtures for FitNesse.slim
I continue working on these. I was stuck in the airport for 5 hours. Between that and the actual flight, I managed to create three different test examples against a C++ RpnCalculator. Each example uses a different kind of fixture. I had a request from @lrojas to publish some results on the blog. So this is that, however these are in progress and rough.
I’m still trying different forms to figure out what I like the best.
By the way, that lastValue stuff in the fixtures has to do with the fact that all of the hook methods return a char* but I’m responsible for cleaning up after myself.
A Decision Table
!|ExecuteBinaryOperator |
|lhs|rhs|operator|expected?|
|3 |4 |- |-1 |
|5 |6 |* |30 |And Its Fixture Code
#include <stdlib.h>
#include <stdio.h>
#include <string>
#include "RpnCalculator.h"
#include "OperationFactory.h"
#include "Fixtures.h"
#include "SlimUtils.h"
struct ExecuteBinaryOperator {
ExecuteBinaryOperator() {
lastValue[0] = 0;
}
int execute() {
RpnCalculator calculator(factory);
calculator.enterNumber(lhs);
calculator.enterNumber(rhs);
calculator.executeOperator(op);
return calculator.getX();
}
static ExecuteBinaryOperator* From(void *fixtureStorage) {
return reinterpret_cast<ExecuteBinaryOperator*>(fixtureStorage);
}
OperationFactory factory;
int lhs;
int rhs;
std::string op;
char lastValue[32];
};
extern "C" {
void* ExecuteBinaryOperator_Create(StatementExecutor* errorHandler, SlimList* args) {
return new ExecuteBinaryOperator;
}
void ExecuteBinaryOperator_Destroy(void* self) {
delete ExecuteBinaryOperator::From(self);
}
static char* setLhs(void* fixture, SlimList* args) {
ExecuteBinaryOperator *self = ExecuteBinaryOperator::From(fixture);
self->lhs = getFirstInt(args);
return self->lastValue;
}
static char* setRhs(void* fixture, SlimList* args) {
ExecuteBinaryOperator *self = ExecuteBinaryOperator::From(fixture);
self->rhs = getFirstInt(args);
return self->lastValue;
}
static char* setOperator(void *fixture, SlimList* args) {
ExecuteBinaryOperator *self = ExecuteBinaryOperator::From(fixture);
self->op = getFirstString(args);
return self->lastValue;
}
static char* expected(void* fixture, SlimList* args) {
ExecuteBinaryOperator *self = ExecuteBinaryOperator::From(fixture);
int result = self->execute();
snprintf(self->lastValue, sizeof(self->lastValue), "%d", result);
return self->lastValue;
}
SLIM_CREATE_FIXTURE(ExecuteBinaryOperator)
SLIM_FUNCTION(setLhs)
SLIM_FUNCTION(setRhs)
SLIM_FUNCTION(setOperator)
SLIM_FUNCTION(expected)
SLIM_END
}A Script Table
!|script |ProgramTheCalculator |
|startProgramCalled|primeFactorsOfSum |
|addOperation |sum |
|addOperation |primeFactors |
|saveProgram |
|enter |4 |
|enter |13 |
|enter |7 |
|execute |primeFactorsOfSum |
|check |stackHas|3|then|2|then|2|then|2|is|true|And Its Fixture Code
#include <stdlib.h>
#include <stdio.h>
#include <string>
#include "RpnCalculator.h"
#include "OperationFactory.h"
#include "SlimUtils.h"
#include "SlimList.h"
#include "Fixtures.h"
struct ProgramTheCalculator {
ProgramTheCalculator() : calculator(factory) {
}
static ProgramTheCalculator* From(void *fixtureStorage) {
return reinterpret_cast<ProgramTheCalculator*>(fixtureStorage);
}
OperationFactory factory;
RpnCalculator calculator;
};
extern "C" {
void* ProgramTheCalculator_Create(StatementExecutor* errorHandler, SlimList* args) {
return new ProgramTheCalculator;
}
void ProgramTheCalculator_Destroy(void *fixture) {
delete ProgramTheCalculator::From(fixture);
}
static char* startProgramCalled(void *fixture, SlimList *args) {
auto *self = ProgramTheCalculator::From(fixture);
self->calculator.createProgramNamed(getFirstString(args));
return remove_const("");
}
static char* addOperation(void *fixture, SlimList *args) {
auto *self = ProgramTheCalculator::From(fixture);
self->calculator.addOperation(getFirstString(args));
return remove_const("");
}
static char* saveProgram(void *fixture, SlimList *args) {
auto *self = ProgramTheCalculator::From(fixture);
self->calculator.saveProgram();
return remove_const("");
}
static char* enter(void *fixture, SlimList *args) {
auto *self = ProgramTheCalculator::From(fixture);
self->calculator.enterNumber(getFirstInt(args));
return remove_const("");
}
static char* execute(void *fixture, SlimList *args) {
auto *self = ProgramTheCalculator::From(fixture);
self->calculator.executeOperator(getFirstString(args));
return remove_const("");
}
static char* stackHasThenThenThenIs(void *fixture, SlimList *args) {
auto *self = ProgramTheCalculator::From(fixture);
for(int i = 0; i < 4; ++i) {
if(self->calculator.getX() != getIntAt(args, i))
return remove_const("false");
self->calculator.executeOperator("drop");
}
return remove_const("true");
}
SLIM_CREATE_FIXTURE(ProgramTheCalculator)
SLIM_FUNCTION(startProgramCalled)
SLIM_FUNCTION(addOperation)
SLIM_FUNCTION(saveProgram)
SLIM_FUNCTION(enter)
SLIM_FUNCTION(execute)
SLIM_FUNCTION(stackHasThenThenThenIs)
SLIM_END
}The Dreaded Query Table
It’s a bit of a pain to produce query results. So much so, I wrote a simple library in Java to make it easier. I can create a well-formed query result from a single object or a list of objects and even do basic transforms (in names and in paths to data). I started using the jakarta bean utils, but my use was so simple (2 methods), I ripped out that library and just hand-wrote the methods I needed. It was not a case of “not invented here syndrom.” I started by using the library, and I had tests. I didn’t like the size of the library relative to how much I was using it, so I just got rid of it.Well here I am working C++ and I felt compelled to make it easier work with query results in C++.
First the FitNesse table, then the fixture and finally the support class. I have tests for it as well, I’m not going to show those, however.!|Query: SingleCharacterNameOperators|
|op |
|+ |
|* |
|/ |
|! |
|- |And Its Fixture Code
#include <stdlib.h>
#include <stdio.h>
#include <vector>
#include <string>
#include <memory>
#include "RpnCalculator.h"
#include "OperationFactory.h"
#include "Fixtures.h"
#include "SlimUtils.h"
#include "QueryResultAccumulator.h"
struct SingleCharacterNameOperators {
OperationFactory factory;
RpnCalculator calculator;
SingleCharacterNameOperators() :
calculator(factory), result(0) {
}
~SingleCharacterNameOperators() {
delete result;
}
static SingleCharacterNameOperators* From(void *fixtureStorage) {
return reinterpret_cast<SingleCharacterNameOperators*> (fixtureStorage);
}
void resetResult(char *newResult) {
delete result;
result = newResult;
}
void conditionallyAddOperatorNamed(const std::string &name) {
if (name.size() == 1) {
accumulator.addFieldNamedWithValue("op", name);
accumulator.finishCurrentObject();
}
}
void buildResult() {
v_string names = calculator.allOperatorNames();
buildResult(names);
}
void buildResult(v_string &names) {
for (v_string::iterator iter = names.begin(); iter != names.end(); ++iter)
conditionallyAddOperatorNamed(*iter);
resetResult(accumulator.produceFinalResults());
}
QueryResultAccumulator accumulator;
char *result;
};
extern "C" {
void* SingleCharacterNameOperators_Create(StatementExecutor* errorHandler,
SlimList* args) {
return new SingleCharacterNameOperators;
}
void SingleCharacterNameOperators_Destroy(void *fixture) {
delete SingleCharacterNameOperators::From(fixture);
}
static char* query(void *fixture, SlimList *args) {
auto *self = SingleCharacterNameOperators::From(fixture);
self->buildResult();
return self->result;
}
SLIM_CREATE_FIXTURE(SingleCharacterNameOperators)
SLIM_FUNCTION(query)SLIM_END
SLIM_ENDAnd the Helper Class
QueryResultAccumulator.h#pragma once
#ifndef QUERYRESULTACCUMULATOR_H_
#define QUERYRESULTACCUMULATOR_H_
class SlimList;
#include <vector>
#include <string>
class QueryResultAccumulator {
public:
typedef std::vector<SlimList*> v_SlimList;
typedef v_SlimList::iterator iterator;
QueryResultAccumulator();
virtual ~QueryResultAccumulator();
void finishCurrentObject();
void addFieldNamedWithValue(const std::string &name, const std::string &value);
char *produceFinalResults();
private:
SlimList* allocate();
void releaseAll();
void setInitialConditions();
private:
v_SlimList created;
SlimList *list;
SlimList *currentObject;
int lastFieldCount;
int currentFieldCount;
char *result;
private:
QueryResultAccumulator(const QueryResultAccumulator&);
QueryResultAccumulator& operator=(const QueryResultAccumulator&);
};
#endif
I know there are too many fields. The counts help with validating correct usage. I also wrote it so one instance could be re-used and I tried to make sure it was in a “ready to receive fields” state when necessary. In any case, this error checking helped find a defect I introduced while refactoring.
QueryResultAccumulator.cpp#include "QueryResultAccumulator.h"
#include "DifferentFieldCountsInObjects.h"
#include "InvalidStateException.h"
extern "C" {
#include "SlimList.h"
#include "SlimListSerializer.h"
}
QueryResultAccumulator::QueryResultAccumulator() : result(0) {
setInitialConditions();
}
QueryResultAccumulator::~QueryResultAccumulator() {
releaseAll();
SlimList_Release(result);
}
void QueryResultAccumulator::setInitialConditions() {
releaseAll();
list = allocate();
currentObject = allocate();
lastFieldCount = -1;
currentFieldCount = -1;
}
SlimList* QueryResultAccumulator::allocate() {
SlimList *list = SlimList_Create();
created.push_back(list);
return list;
}
void QueryResultAccumulator::releaseAll() {
for (iterator i = created.begin(); i != created.end(); ++i)
SlimList_Destroy(*i);
created.clear();
}
void QueryResultAccumulator::finishCurrentObject() {
if(lastFieldCount >= 0 && lastFieldCount != currentFieldCount)
throw DifferentFieldCountsInObjects(lastFieldCount, currentFieldCount);
SlimList_AddList(list, currentObject);
currentObject = allocate();
lastFieldCount = currentFieldCount;
currentFieldCount = -1;
}
void QueryResultAccumulator::addFieldNamedWithValue(const std::string &name, const std::string &value) {
SlimList *fieldList = allocate();
SlimList_AddString(fieldList, name.c_str());
SlimList_AddString(fieldList, value.c_str());
SlimList_AddList(currentObject, fieldList);
++currentFieldCount;
}
char* QueryResultAccumulator::produceFinalResults() {
if(currentFieldCount != -1)
throw InvalidStateException("Current object not written");
SlimList_Release(result);
result = SlimList_Serialize(list);
setInitialConditions();
return result;
}void SlimList_Release(char *serializedResults);void SlimList_Release(char *serializedResults)
{
if(serializedResults)
free(serializedResults);
}I needed to add these methods due to a false-positive memory leak indicated when using CppUTest to test this code. That’s another blog.
FitNesse, C++ and cslim, step-by-step instructions 2
Title says it all: http://schuchert.wikispaces.com/cpptraining.GettingStartedWithFitNesseInCpp
First draft. If you have problems, please let me know.
A Few C plus plus TDD videos 1
Using CppUTest, gcc 4.4 and the Eclipse CDT.
Rough, as usual.
Might redo first one with increased font size. Considering redoing whole series at 800×600.
C++ Algorithms, Boost and function currying 38
I’ve been experimenting with C++ using the Eclipse CDT and gcc 4.4. Since I’m a fan of boost, I’ve been using that as well. I finally got into I realistic use of boost::bind.
I converted this:int Dice::total() const {
int total = 0;
for(const_iterator current = dice.begin();
current != dice.end();
++current)
total += (*current)->faceValue();
return total;
}int Dice::total() const {
return std::accumulate(
dice.begin(),
dice.end(),
0,
bind(std::plus<int>(), _1, bind(&Die::faceValue, _2))
);
}To see how to go from the first version to the final version with lots of steps in between: http://schuchert.wikispaces.com/cpptraining.SummingAVector.
This is a first draft. I’ll be cleaning it up over the next few days. If you see typos, or if anything is not clear from the code, please let me know where. Also, if my interpretation of what boost is doing under the covers (there’s not much of that) is wrong, please correct me.
Thanks!
Hello World Revisited 16
Surprising revelations while taking a TDD approach to writing hello world.
Here it nearly 21 years since I started writing in C++ (and more for C+) and I realize I’ve been blindly writing main functions to actually do something.
This insanity must stop!
What am I talking about? Read it here.
TDD is wasting time if... 48
You have no design sense.
OK, discuss.
Parts 5 and 6 of the 4-part series 14
The title says it all. I was bothered by a few things in the Shunting Yard Algorithm (actually many more things), but I felt compelled to fix two of those things.
So if you have a look at the album, you’ll notice 2 more videos:- Video 5 of 4: Remove the need for spaces between tokens.
- Video 6 of 4: Remove duplication of operators in algorithm and tokenizer.
Hope these are interesting or at least entertaining.
C# TDD Videos - You asked for them 18
Several people asked for them, so here is a series of 4 videos. The first series on the RPN calculator in Java was a bit rough, these are even rougher.
Even so, hope you find them valuable.
Shunting Yard Algorithm in C# Video Album
Comments and feedback welcome.
Open Spaces at conferences, what's your take? 12
I’m the host for an open space this year at Agile Testing Days 2010, Berlin (October 4th – 7th, the open space being the 7th). I’ve attended a few open spaces and I have some ideas on what the role of host might entail.
But, I know I’m better at collecting requirements than sourcing them, so what have you experienced that worked. What has not worked. Any advice you want to give me?
Please help me better understand what I can do to facilitate a successful open space.
What questions should I be asking?
Do you think having a few things scheduled up front in one room, and several unscheduled rooms left to be determined October 4th – 6th, through a a mix if you will, is an OK thing to do, or should what happens be left completely open?
Or, leave everything completely open until the 7th, then start with the “traditional” introductions and let the agenda form then an there?
I’m aware of a few models of open spaces. What I really want to know is, what works for you.