10 October 2021, 12:43 | #1 |
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
Junior C++ developer questions - templates
I'm trying to learn C++ and I'm currently trying to get my head around templates. I'm porting some C code I wrote a while back that handles object pooling and queuing. This is what I have at the moment:
queue.h Code:
#pragma once #include <exec/types.h> namespace data_structures { // Queue, implemented as a circular buffer. template <typename type, UWORD maxLength> class Queue { public: Queue(type * * contents); BOOL enqueue(type * element); type * dequeue(); private: type * * contents; UWORD length; UWORD readIndex; UWORD writeIndex; }; template <typename type, UWORD maxLength> Queue<type, maxLength>::Queue(type * * contents) : contents(contents), length(0), readIndex(0), writeIndex(0) { } template <typename type, UWORD maxLength> BOOL Queue<type, maxLength>::enqueue(type * element) { if (length > maxLength) return FALSE; contents[writeIndex] = element; length++; writeIndex++; if (writeIndex == maxLength) writeIndex = 0; return TRUE; } template <typename type, UWORD maxLength> type * Queue<type, maxLength>::dequeue() { if (length == 0) return NULL; type * element = contents[readIndex]; length--; readIndex++; if (readIndex == maxLength) readIndex = 0; return element; } } If anyone wants to give constructive criticism of my code or my approach I'd really appreciate it. My Pool class, which is what I need for the next step in the actual project I'm working on is just going to be a layer on top of this Queue. |
10 October 2021, 14:38 | #2 |
Registered User
Join Date: Feb 2017
Location: Denmark
Posts: 1,107
|
It looks like you have an off-by-one error in your enqueue function (the check should be length >= maxLength, or just length == maxLength).
You may want to write some tests In C++ you'd normally have the class manage resources (in this case memory). Having to alloc/free memory from the outside is error-prone (you face lifetime issues, possible memory leaks and have to make sure you adjust both the maxLength template argument and allocation size when making changes). I'd just have an array as a data member in the class (or handle allocation in the constructor and freeing in the destructor). You probably also want to prohibit copying of the queue (or make sure you handle it properly). Your queue type currently only handles pointers, and that's fine. If you want it to be more generic, be aware that there will be some issues with your current interface (namely getting dequeue right), see http://www.gotw.ca/gotw/008.htm. This is the reason std::queue from the standard library splits the operation/check up into three methods: empty/front and pop. Even if you don't care about exception safety, your queue has the (admittedly minor) issue that if you enqueue a "NULL", later on when you dequeue it, you won't know whether it was because you got a NULL or the queue was empty. |
10 October 2021, 14:39 | #3 | |
Registered User
Join Date: Jan 2019
Location: Germany
Posts: 3,233
|
Quote:
Amiga is not exactly a C++ friendly environment, but those C++ compilers that are full C++ compilers (that is, not SAS/C) come with a STL port. Good C++ style would be to use STL when possible, that is, avoid to reinvent the wheel. You find queue classes there that are ready to use. If you want advice on how to write C++, then this is not the right forum. I suggest reading some books, or check comp.lang.c++.moderated (the real C++ experts are there). For example, I would recommend "Excepitonal C++" by Herb Sutter as a good collection of exercises and examples. |
|
10 October 2021, 15:28 | #4 | |||
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
Quote:
Code:
if (queue->length == queue->_maxLength) return FALSE; Quote:
Code:
class SpanPool : public Pool<Span, spanPoolSize> { public: SpanPool(); protected: Span * * createContents(); }; SpanPool::SpanPool() : Pool(createContents()) { } Span * * SpanPool::createContents() { Span * spans = new Span[spanPoolSize]; Span * * spanPoolContents = new Span * [spanPoolSize]; for (UWORD i = 0; i < spanPoolSize; i++) spanPoolContents[i] = &spans[i]; return spanPoolContents; } As for prohibiting copying of the queue, I guess you mean deleting the default copy constructors? I know of that, but not quite enough to implement it yet. Quote:
I'll read that link properly. I don't have exceptions enabled, but I might learn something in any case. |
|||
10 October 2021, 15:36 | #5 |
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
|
10 October 2021, 16:07 | #6 |
Registered User
Join Date: Jan 2019
Location: Germany
Posts: 3,233
|
Sorry, but this doesn't make things less true, and I surely didn't want to upset you. There are really many better places to collect advice for C++, from people with a much stronger background in C++. I provided links to at least two resources where you find more professional advice than here.
|
10 October 2021, 16:18 | #7 |
This cat is no more
Join Date: Dec 2004
Location: FRANCE
Age: 52
Posts: 8,200
|
there is stackoverflow but you need to focus on your exact problem. you cannot just dump your code and expect people to debug it.
|
10 October 2021, 16:22 | #8 | |
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
Quote:
|
|
10 October 2021, 16:23 | #9 |
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
|
10 October 2021, 17:07 | #10 |
This cat is no more
Join Date: Dec 2004
Location: FRANCE
Age: 52
Posts: 8,200
|
in that case you can submit your code to https://codereview.stackexchange.com/
(and cut the aggressivity, really, people here are trying to help, maybe they read too quickly but they're not trying to talk down to you) |
10 October 2021, 17:16 | #11 | |
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
Quote:
|
|
10 October 2021, 17:25 | #12 |
Registered User
Join Date: Jun 2020
Location: Druidia
Posts: 389
|
Deimos
Your code looks perfectly reasonable as a use of templates and classes to generalize the code and give it a nicer interface than straight C. There’s really no other magic C++ or the standard libraries have that you’re missing out on. Most games are written in C with some C++ features and almost never using the standard libraries. Tools can blur the line and use the standard libraries where tightly controlling performance and resources are less important. You’ll hear a lot of dogma about how you are supposed to use C++ but you can safely ignore all that and just use what features are actually helpful as you already have! |
10 October 2021, 17:40 | #13 | |
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
Quote:
I've made some of the changes suggested by paraj and I'm going to write a few tests. This code should help me progress my span buffer experiments. |
|
10 October 2021, 18:03 | #14 |
Registered User
Join Date: Jun 2020
Location: Druidia
Posts: 389
|
One thing that occurs to me is that if you are implementing a pool of objects then a potentially better approach would be a free list. Since only free objects need to be in the list you can just overlay a next pointer or index. Meaning you won’t waste space. You can maybe use a Union or just some casts to access the next pointer. I can dig up an example later if you like.
|
10 October 2021, 18:04 | #15 | |
Registered User
Join Date: Jan 2019
Location: Germany
Posts: 3,233
|
Quote:
Concerning your code, your queue only stores pointers, not arbitrary types, which is a restriction, and instead of returning booleans as success indicators, a C++-ish way of doing such things is to throw exceptions in exceptional situations. Concerning learning material, I didn't mention "Exceptional C++" out of nowhere. It is really a good book, and while it does not discuss a queue class, it has a lengthly and very instructive discussion of a stack class by one of the best C++ insiders, with many of the pitfalls one can trap into. Thus, I'm sorry, but I can only repeat myself. If you want to learn some serious C++, please check some of the sources I provided. This is really not quite the right place. If you don't, and only need a queue, well, as said, the STL works perfectly fine for such cases, it's made for exactly that. It is part of any C++ compiler I know of, as it is also part of the C++ ISO standard (for exactly that purpose). |
|
10 October 2021, 18:30 | #16 |
Registered User
Join Date: Jun 2020
Location: Druidia
Posts: 389
|
Just to add more game developer perspective. Almost no games use exceptions either. They add a global performance cost not worth paying. Also in a game you don’t really want to handle problems afterward. You want to anticipate them ahead of time and avoid them.
|
10 October 2021, 18:47 | #17 | |||||
It's coming back!
Join Date: Jul 2018
Location: comp.sys.amiga
Posts: 762
|
Quote:
Quote:
Exceptions are not enabled for me. Quote:
Quote:
Quote:
|
|||||
10 October 2021, 19:06 | #18 | |||
Registered User
Join Date: Jan 2019
Location: Germany
Posts: 3,233
|
Quote:
Exceptions are also part of C++ since a long time. Quote:
Quote:
You also asked for critique, and here is mine. If you don't want to listen, and don't want to learn C++, then please don't shoot the messenger. Your class is not quite "up to style", and not quite the way how a C++ is generally understood to be used. |
|||
10 October 2021, 19:20 | #19 | |
Registered User
Join Date: Feb 2017
Location: Denmark
Posts: 1,107
|
First let me say that experimenting and reinventing the wheel is a great way to learn, so definitely keep doing that! Since I don't know what you're trying to achieve it's hard to give specific recommendations, but I think you may be too trying to go too generic too soon. You probably don't need to actually support arbitrary allocation strategies for example like the standard library (tries) to support via it's Allocator type parameter to containers.
It also seems like you're making quite a number of dynamic memory allocations, maybe that's necessary for what you're doing, but it's something I'd consider reviewing. My general recommendation would be to start by creating concrete classes (i.e. not specialized) that do what you actually need in your program, and only start templating once the need arises. Say a Span* fixed size queue that holds 512 elements. When it turns out you need more than one of those and it needs another size: That's when you convert your class to a templated one. Later on you might (or not and you've saved a bunch of work and debugging) need a fixed size queue of chars, and you add the type as a template parameter and consolidate. Of course feel free to skip a step or two if you know in advance that you actually need the flexibility, but be careful in jumping directly to a full generic solution. YAGNI. You can always refactor your code later on to avoid code duplication. A fixed sized queue is BTW a great example of something the standard library doesn't supply, and where it can make sense to roll your own (or use someone else's work). Also consider if you can use something like EASTL (https://github.com/electronicarts/EA.../include/EASTL). Quote:
Code:
public: Queue(const Queue&) = delete; // No copy Queue& operator=(const Queue&) = delete; // No assignment Code:
private: // Never define these so you will get a linker error if someone tries to use them Queue(const Queue&); // No copy Queue& operator=(const Queue&); // No assignment BTW Thomas' suggestion of "Exceptional C++" is indeed good along with the followup. The recommendations in it are relevant even (or especially) if you're using a compiler from the 90ies. The GOTW article I linked (that Thomas also references) forms the basis for one of the chapters in the book. |
|
10 October 2021, 21:24 | #20 | |
This cat is no more
Join Date: Dec 2004
Location: FRANCE
Age: 52
Posts: 8,200
|
Quote:
not trying to talk down to you. Really. At stackoverflow, a lot of people misunderstand how the platform work and dump their code without trying to create a minimal example. On EAB, among fellow amigans, there is no such rule, and people CAN debug your code dump without complaining, it happens all the time with asm tricky issues and such. But as said somewhere else you'd be better off somewhere where people know C++, not amiga. I see there are already good suggestions. I'm off now. |
|
Currently Active Users Viewing This Thread: 1 (0 members and 1 guests) | |
Thread Tools | |
Similar Threads | ||||
Thread | Thread Starter | Forum | Replies | Last Post |
Junior's HOL Contributions | Junior | HOL contributions | 26 | 22 February 2020 21:09 |
Jumpman Junior - Problem | hipoonios | support.WinUAE | 22 | 31 December 2016 00:43 |
Jetstrike Junior - Coverdisk? | TCD | HOL data problems | 3 | 28 July 2010 23:35 |
ADI Junior is a false entry | Another World | HOL data problems | 2 | 25 September 2008 19:20 |
Junior Artist and Educational Software | Retro1234 | request.Old Rare Games | 0 | 07 June 2007 22:45 |
|
|