Improve Vector and HttpHeaders iteration (#2745)

This PR improves use of C++ iterator pattern instead of traditional `for(i=0; i<list.count(); ++i)` approach. **Template Vector `indexOf` methods** Allows comparison with things other than an object instance, e.g. its name, depending on operator overloads. Instead of recoding a comparison loop, just implement the appropriate `operator==` for the classes and use `Vector::indexOf`. **Enable iteration for HttpHeaders** This is based on HashMap but stores an enumeration for the key as it's more efficient. HashMap iteration returns a proxy object, so here we provide a customised version with an implicit `String()` operator. This makes for simpler usage.
SmingHub · Mar 26, 2024 · 2db98d5 · 2db98d5
1 parent 7af1fac
commit 2db98d5
Show file tree

Hide file tree

Showing 22 changed files with 140 additions and 74 deletions.
diff --git a/Sming/Components/Hosted/include/Hosted/Client.h b/Sming/Components/Hosted/include/Hosted/Client.h
@@ -154,8 +154,8 @@ class Client
 
 		host_debug_i("Available commands:");
 
-		for(size_t i = 0; i < commands.count(); i++) {
-			host_debug_i("\t%s => %d", commands.keyAt(i).c_str(), commands.valueAt(i));
+		for(auto cmd : commands) {
+			host_debug_i("\t%s => %d", cmd.key().c_str(), cmd.value());
 		}
 
 		host_debug_i("Connected. Starting application");

diff --git a/Sming/Components/Network/src/Network/Http/HttpClientConnection.cpp b/Sming/Components/Network/src/Network/Http/HttpClientConnection.cpp
@@ -367,9 +367,9 @@ void HttpClientConnection::sendRequestHeaders(HttpRequest* request)
 		request->headers[HTTP_HEADER_TRANSFER_ENCODING] = _F("chunked");
 	}
 
-	for(unsigned i = 0; i < request->headers.count(); i++) {
+	for(auto hdr : request->headers) {
 		// TODO: add name and/or value escaping (implement in HttpHeaders)
-		sendString(request->headers[i]);
+		sendString(hdr);
 	}
 	sendString("\r\n");
 }

diff --git a/Sming/Components/Network/src/Network/Http/HttpHeaders.cpp b/Sming/Components/Network/src/Network/Http/HttpHeaders.cpp
@@ -11,6 +11,25 @@
 #include "HttpHeaders.h"
 #include <debug_progmem.h>
 
+String HttpHeaders::HeaderConst::getFieldName() const
+{
+	return fields.toString(key());
+}
+
+size_t HttpHeaders::HeaderConst::printTo(Print& p) const
+{
+	size_t n{0};
+	n += p.print(getFieldName());
+	n += p.print(" = ");
+	n += p.print(value());
+	return n;
+}
+
+HttpHeaders::HeaderConst::operator String() const
+{
+	return fields.toString(key(), value());
+}
+
 const String& HttpHeaders::operator[](const String& name) const
 {
 	auto field = fromString(name);
@@ -40,9 +59,7 @@ bool HttpHeaders::append(const HttpHeaderFieldName& name, const String& value)
 
 void HttpHeaders::setMultiple(const HttpHeaders& headers)
 {
-	for(unsigned i = 0; i < headers.count(); i++) {
-		HttpHeaderFieldName fieldName = headers.keyAt(i);
-		auto fieldNameString = headers.toString(fieldName);
-		operator[](fieldNameString) = headers.valueAt(i);
+	for(auto hdr : headers) {
+		operator[](hdr.getFieldName()) = hdr.value();
 	}
 }
diff --git a/Sming/Components/Network/src/Network/Http/HttpHeaders.h b/Sming/Components/Network/src/Network/Http/HttpHeaders.h
@@ -34,13 +34,61 @@
 class HttpHeaders : public HttpHeaderFields, private HashMap<HttpHeaderFieldName, String>
 {
 public:
+	class HeaderConst : public BaseElement<true>
+	{
+	public:
+		HeaderConst(const HttpHeaderFields& fields, const HttpHeaderFieldName& key, const String& value)
+			: BaseElement(key, value), fields(fields)
+		{
+		}
+
+		String getFieldName() const;
+		operator String() const;
+		size_t printTo(Print& p) const;
+
+	private:
+		const HttpHeaderFields& fields;
+	};
+
+	class Iterator : public HashMap::Iterator<true>
+	{
+	public:
+		Iterator(const HttpHeaders& headers, unsigned index)
+			: HashMap::Iterator<true>::Iterator(headers, index), fields(headers)
+		{
+		}
+
+		HeaderConst operator*()
+		{
+			return HeaderConst(fields, map.keyAt(index), map.valueAt(index));
+		}
+
+		HeaderConst operator*() const
+		{
+			return HeaderConst(fields, map.keyAt(index), map.valueAt(index));
+		}
+
+	private:
+		const HttpHeaderFields& fields;
+	};
+
 	HttpHeaders() = default;
 
 	HttpHeaders(const HttpHeaders& headers)
 	{
 		*this = headers;
 	}
 
+	Iterator begin() const
+	{
+		return Iterator(*this, 0);
+	}
+
+	Iterator end() const
+	{
+		return Iterator(*this, count());
+	}
+
 	using HashMap::operator[];
 
 	/** @brief Fetch a reference to the header field value by name
@@ -70,6 +118,11 @@ class HttpHeaders : public HttpHeaderFields, private HashMap<HttpHeaderFieldName
 		return toString(keyAt(index), valueAt(index));
 	}
 
+	template <bool is_const> String operator[](const BaseElement<is_const>& elem) const
+	{
+		return toString(elem.key(), elem.value());
+	}
+
 	using HashMap::contains;
 
 	/**

diff --git a/Sming/Components/Network/src/Network/Http/HttpRequest.cpp b/Sming/Components/Network/src/Network/Http/HttpRequest.cpp
@@ -77,8 +77,8 @@ String HttpRequest::toString() const
 	if(!headers.contains(HTTP_HEADER_HOST)) {
 		content += headers.toString(HTTP_HEADER_HOST, uri.getHostWithPort());
 	}
-	for(unsigned i = 0; i < headers.count(); i++) {
-		content += headers[i];
+	for(auto hdr : headers) {
+		content += hdr;
 	}
 
 	if(!headers.contains(HTTP_HEADER_CONTENT_LENGTH)) {

diff --git a/Sming/Components/Network/src/Network/Http/HttpResponse.cpp b/Sming/Components/Network/src/Network/Http/HttpResponse.cpp
@@ -137,8 +137,8 @@ String HttpResponse::toString() const
 	content += ' ';
 	content += ::toString(code);
 	content += " \r\n";
-	for(unsigned i = 0; i < headers.count(); i++) {
-		content += headers[i];
+	for(auto hdr : headers) {
+		content += hdr;
 	}
 	content += "\r\n";
 

diff --git a/Sming/Components/Network/src/Network/Http/HttpServerConnection.cpp b/Sming/Components/Network/src/Network/Http/HttpServerConnection.cpp
@@ -312,8 +312,8 @@ void HttpServerConnection::sendResponseHeaders(HttpResponse* response)
 		response->headers[HTTP_HEADER_DATE] = DateTime(SystemClock.now(eTZ_UTC)).toHTTPDate();
 	}
 
-	for(unsigned i = 0; i < response->headers.count(); i++) {
-		sendString(response->headers[i]);
+	for(auto hdr : response->headers) {
+		sendString(hdr);
 	}
 	sendString("\r\n");
 }

diff --git a/Sming/Components/Network/src/Network/Http/Websocket/WebsocketConnection.cpp b/Sming/Components/Network/src/Network/Http/Websocket/WebsocketConnection.cpp
@@ -292,9 +292,9 @@ void WebsocketConnection::broadcast(const char* message, size_t length, ws_frame
 	memcpy(copy, message, length);
 	std::shared_ptr<const char> data(copy, [](const char* ptr) { delete[] ptr; });
 
-	for(unsigned i = 0; i < websocketList.count(); i++) {
+	for(auto skt : websocketList) {
 		auto stream = new SharedMemoryStream<const char>(data, length);
-		websocketList[i]->send(stream, type);
+		skt->send(stream, type);
 	}
 }
 

diff --git a/Sming/Components/Network/src/Network/SmtpClient.cpp b/Sming/Components/Network/src/Network/SmtpClient.cpp
@@ -306,8 +306,8 @@ void SmtpClient::sendMailHeaders(MailMessage* mail)
 		mail->stream = mStream;
 	}
 
-	for(unsigned i = 0; i < mail->headers.count(); i++) {
-		sendString(mail->headers[i]);
+	for(auto hdr : mail->headers) {
+		sendString(hdr);
 	}
 	sendString("\r\n");
 }

diff --git a/Sming/Components/Network/src/Network/TcpServer.cpp b/Sming/Components/Network/src/Network/TcpServer.cpp
@@ -164,13 +164,10 @@ void TcpServer::shutdown()
 		return;
 	}
 
-	for(unsigned i = 0; i < connections.count(); i++) {
-		TcpConnection* connection = connections[i];
-		if(connection == nullptr) {
-			continue;
+	for(auto& connection : connections) {
+		if(connection != nullptr) {
+			connection->setTimeOut(1);
 		}
-
-		connection->setTimeOut(1);
 	}
 }
 

diff --git a/Sming/Core/Data/ObjectMap.h b/Sming/Core/Data/ObjectMap.h
@@ -208,7 +208,7 @@ template <typename K, typename V> class ObjectMap
 	 */
 	void set(const K& key, V* value)
 	{
-		int i = indexOf(key);
+		int i = entries.indexOf(key);
 		if(i >= 0) {
 			entries[i].value.reset(value);
 		} else {
@@ -224,7 +224,7 @@ template <typename K, typename V> class ObjectMap
 	 */
 	V* find(const K& key) const
 	{
-		int index = indexOf(key);
+		int index = entries.indexOf(key);
 		return (index < 0) ? nullptr : entries[index].value.get();
 	}
 
@@ -235,12 +235,7 @@ template <typename K, typename V> class ObjectMap
 	 */
 	int indexOf(const K& key) const
 	{
-		for(unsigned i = 0; i < entries.count(); i++) {
-			if(entries[i].key == key) {
-				return i;
-			}
-		}
-		return -1;
+		return entries.indexOf(key);
 	}
 
 	/**
@@ -250,7 +245,7 @@ template <typename K, typename V> class ObjectMap
 	 */
 	bool contains(const K& key) const
 	{
-		return indexOf(key) >= 0;
+		return entries.contains(key);
 	}
 
 	/**
@@ -272,10 +267,9 @@ template <typename K, typename V> class ObjectMap
 		int index = indexOf(key);
 		if(index < 0) {
 			return false;
-		} else {
-			removeAt(index);
-			return true;
 		}
+		removeAt(index);
+		return true;
 	}
 
 	/**
@@ -322,6 +316,11 @@ template <typename K, typename V> class ObjectMap
 		K key;
 		std::unique_ptr<V> value;
 
+		bool operator==(const K& keyToFind) const
+		{
+			return key == keyToFind;
+		}
+
 		Entry(const K& key, V* value) : key(key)
 		{
 			this->value.reset(value);

diff --git a/Sming/Core/Data/ObjectQueue.h b/Sming/Core/Data/ObjectQueue.h
@@ -31,11 +31,11 @@ template <typename T, int rawSize> class ObjectQueue : public FIFO<T*, rawSize>
 
 	T* peek() const
 	{
-		return FIFO<T*, rawSize>::count() ? FIFO<T*, rawSize>::peek() : nullptr;
+		return this->count() ? FIFO<T*, rawSize>::peek() : nullptr;
 	}
 
 	T* dequeue()
 	{
-		return FIFO<T*, rawSize>::count() ? FIFO<T*, rawSize>::dequeue() : nullptr;
+		return this->count() ? FIFO<T*, rawSize>::dequeue() : nullptr;
 	}
 };
diff --git a/Sming/Libraries/DiskStorage b/Sming/Libraries/DiskStorage
diff --git a/Sming/Libraries/Yeelight/YeelightBulb.cpp b/Sming/Libraries/Yeelight/YeelightBulb.cpp
@@ -49,11 +49,11 @@ void YeelightBulb::sendCommand(const String& method, const Vector<String>& param
 	doc["id"] = requestId++;
 	doc["method"] = method;
 	auto arr = doc.createNestedArray("params");
-	for(unsigned i = 0; i < params.count(); i++) {
-		if(isNumeric(params[i])) {
-			arr.add(params[i].toInt());
+	for(auto& param : params) {
+		if(isNumeric(param)) {
+			arr.add(param.toInt());
 		} else {
-			arr.add(params[i]);
+			arr.add(param);
 		}
 	}
 	String request = Json::serialize(doc);

diff --git a/Sming/Wiring/WHashMap.h b/Sming/Wiring/WHashMap.h
@@ -165,7 +165,7 @@ template <typename K, typename V> class HashMap
 			return ElementConst{map.keyAt(index), map.valueAt(index)};
 		}
 
-	private:
+	protected:
 		Map& map;
 		unsigned index{0};
 	};

diff --git a/Sming/Wiring/WVector.h b/Sming/Wiring/WVector.h
@@ -113,7 +113,7 @@ template <typename Element> class Vector : public Countable<Element>
 		return _data.size;
 	}
 
-	bool contains(const Element& elem) const
+	template <typename T> bool contains(const T& elem) const
 	{
 		return indexOf(elem) >= 0;
 	}
@@ -127,7 +127,7 @@ template <typename Element> class Vector : public Countable<Element>
 		return _data[0];
 	}
 
-	int indexOf(const Element& elem) const;
+	template <typename T> int indexOf(const T& elem) const;
 
 	bool isEmpty() const
 	{
@@ -143,7 +143,7 @@ template <typename Element> class Vector : public Countable<Element>
 		return _data[_size - 1];
 	}
 
-	int lastIndexOf(const Element& elem) const;
+	template <typename T> int lastIndexOf(const T& elem) const;
 
 	unsigned int count() const override
 	{
@@ -178,9 +178,9 @@ template <typename Element> class Vector : public Countable<Element>
 		_size = 0;
 	}
 
-	bool removeElement(const Element& obj)
+	template <typename T> bool removeElement(const T& elem)
 	{
-		return removeElementAt(indexOf(obj));
+		return removeElementAt(indexOf(elem));
 	}
 
 	/**
@@ -316,7 +316,7 @@ template <class Element> void Vector<Element>::copyInto(Element* array) const
 	}
 }
 
-template <class Element> int Vector<Element>::indexOf(const Element& elem) const
+template <class Element> template <typename T> int Vector<Element>::indexOf(const T& elem) const
 {
 	for(unsigned int i = 0; i < _size; i++) {
 		if(_data[i] == elem) {
@@ -327,7 +327,7 @@ template <class Element> int Vector<Element>::indexOf(const Element& elem) const
 	return -1;
 }
 
-template <class Element> int Vector<Element>::lastIndexOf(const Element& elem) const
+template <class Element> template <typename T> int Vector<Element>::lastIndexOf(const T& elem) const
 {
 	// check for empty vector
 	if(_size == 0) {

diff --git a/samples/Basic_Ssl/app/application.cpp b/samples/Basic_Ssl/app/application.cpp
@@ -40,8 +40,8 @@ int onDownload(HttpConnection& connection, bool success)
 	Serial << _F(", received ") << stream->available() << _F(" bytes") << endl;
 
 	auto& headers = connection.getResponse()->headers;
-	for(unsigned i = 0; i < headers.count(); ++i) {
-		Serial.print(headers[i]);
+	for(auto hdr : headers) {
+		Serial.print(hdr);
 	}
 
 	auto ssl = connection.getSsl();

diff --git a/samples/Basic_WiFi/app/application.cpp b/samples/Basic_WiFi/app/application.cpp
@@ -14,9 +14,9 @@ void listNetworks(bool succeeded, BssList& list)
 		return;
 	}
 
-	for(unsigned i = 0; i < list.count(); i++) {
-		Serial << _F("\tWiFi: ") << list[i].ssid << ", " << list[i].getAuthorizationMethodName();
-		if(list[i].hidden) {
+	for(auto& bss : list) {
+		Serial << _F("\tWiFi: ") << bss.ssid << ", " << bss.getAuthorizationMethodName();
+		if(bss.hidden) {
 			Serial << _F(" (hidden)");
 		}
 		Serial.println();