Code Review Asked by Apoqlite on January 12, 2021
I’m trying to create a node editor “framework” (IDK what to call it, but essentially there is no GUI and should be implemented using these classes), similar to rete.js, blueprints(maybe an exception since there are events involved), imnodes, etc; and am creating a pin class/struct.
A pin, in this case, is the small things you actually connect (those circles on the sides of the actual nodes which can be inputs or outputs).
I’m using a single header, since almost everything I look into recommends using it for templated classes.
I think there are many problems in the code in general, but another thing that has been bugging me is the naming of classes, enums, members, etc., especially the naming of PinType, InputOutputCount (including the members itself), IOCount.
#pragma once
#include <string>
#include <vector>
enum PinType {//Whether or not the pin takes in values from other pins(input) or gives the value to other pins(output). TODO: Name change.
INPUT,
OUTPUT
};
enum InputOutputCount {//How many stuff the pin can take in or give out. TODO: Name change.
NOTHING = 0,//Is this redundant?
ONE = 1,
MULTIPLE = -1
};
struct PinAbstract {//An abstract superclass for the actual Pin class, in order to store it in vectors, since Pin is a templated class.
PinType Type;
InputOutputCount IOCount = ONE;
virtual ~PinAbstract() = 0;//Don't know much about destructors, but this was the only thing that worked for me.
};
template <class T>
struct Pin : public PinAbstract {//The actual pin class
std::string Title = "Title";//The title of the pin
T* Value = nullptr;//The value passed or being given.
std::vector<Pin<T>*> Connections = {};//The other pins this pin is connected to. NOTE: same template type since two connected pins should be of same type. So a string input only takes in from a string output.
};
template <class T>
void ConnectPins(Pin<T> *pin1, Pin<T> *pin2) {//NOTE: same template type for the same reason above.
if (((pin1->Type == OUTPUT && pin2->Type == INPUT) || (pin1->Type == INPUT && pin2->Type == OUTPUT)) && ((int)pin1->IOCount - pin1->Connections.size() != 0 && (int)pin2->IOCount - pin2->Connections.size() != 0)) {
/*Only connect input with output and vice-versa AND make sure that connection doesn't exceed max connections of any one of the pins*/
pin1->Connections.push_back(pin2);
pin2->Connections.push_back(pin1);
}
}
Any sort of help is greatly appreciated. Including things that are not even code, including naming fixes, general strategies, etc.
One thing that you should avoid, if possible, is the use of raw pointers, from my point of view you should convert them on shared pointers by default and once your software evolves check if you need a more specific type, such as a unique pointer. On the other hand, if you can not avoid the use of a raw pointer is fine if you have tests that cover that, but as a good practise I would recommend to use shared/unique pointers by default to avoid memory issues and try to stay away of raw pointers as much as you can.
Answered by camp0 on January 12, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP