Merge lp:~hjd/widelands/class-FileSystem into lp:widelands

[Hans Joachim Desserud]

From my previous patch I got the impression classes were preferred. Therefore, here is a struct converted to class since it was referred to as both in different places.

[Mark Scott (mxsscott)]

Looks fine.

Here's some information on struct vs class. It is purely a stylistic thing, but almost all code I've seen recently (outside of Widelands) uses class for anything that has methods or is vaguely C++ like. struct is only used for things that might be exchanged with C.

http://www.cplusplus.com/forum/beginner/5980/
http://stackoverflow.com/questions/54585/when-should-you-use-a-class-vs-a-struct-in-c

You could go one further and change LayeredFileSystem, ZipFileSystem and the other derived classes of FileSystem.

(There's another reason - have you ever heard of someone talking about derived structs, or substructing vs subclassing?)

review: Approve

[SirVer (sirver)]

lgtm.

some people do not take offense in a struct with constructor to guarantee a certain state or an operator< to make sure it can be sorted properly. As soon as the struct gets non trivial members it should be converted to a class for the reasons mark mentions.

Some devs in the team have used struct for everything to safe the public: line - i think it is easier to just default to class. So yes, more patches of this kind are very welcome.

review: Approve

[Hans Joachim Desserud (hjd)]

Thanks for the info. :) Fixed the of the substructs^W subclasses in the same directory now.

I also saw one of them had multiple access qualifier each assigning the same level. Based on what I could find [1] that was redundant so I removed them. One use case for this would be if it grouped methods, members, etc in separate groups, but that did not seem to be the case here since the the same access level appeared twice in a row.

[1] http://stackoverflow.com/questions/2719641/can-there-be-two-public-section-in-a-class-if-yes-then-why-and-in-which-circu

[SirVer (sirver)]

and thanks again :)

=== modified file 'src/game_io/game_data_packet.h'
--- src/game_io/game_data_packet.h	2012-02-15 21:25:34 +0000
+++ src/game_io/game_data_packet.h	2013-01-11 15:01:25 +0000
@@ -22,7 +22,7 @@
 
 #include "wexception.h"
 
-struct FileSystem;
+class FileSystem;
 
 namespace Widelands {
 

=== modified file 'src/game_io/game_loader.h'
--- src/game_io/game_loader.h	2012-02-15 21:25:34 +0000
+++ src/game_io/game_loader.h	2013-01-11 15:01:25 +0000
@@ -23,7 +23,7 @@
 #include <stdint.h>
 #include <string>
 
-struct FileSystem;
+class FileSystem;
 
 namespace Widelands {
 

=== modified file 'src/game_io/game_saver.h'
--- src/game_io/game_saver.h	2012-02-15 21:25:34 +0000
+++ src/game_io/game_saver.h	2013-01-11 15:01:25 +0000
@@ -22,7 +22,7 @@
 
 #include "wexception.h"
 
-struct FileSystem;
+class FileSystem;
 
 namespace Widelands {
 

=== modified file 'src/graphic/font_handler1.h'
--- src/graphic/font_handler1.h	2012-12-31 11:21:15 +0000
+++ src/graphic/font_handler1.h	2013-01-11 15:01:25 +0000
@@ -29,7 +29,7 @@
 #include "ui_basic/align.h"
 
 class RenderTarget;
-struct FileSystem;
+class FileSystem;
 
 class IPicture;
 class IGraphic;

=== modified file 'src/io/filesystem/disk_filesystem.h'
--- src/io/filesystem/disk_filesystem.h	2012-02-15 21:25:34 +0000
+++ src/io/filesystem/disk_filesystem.h	2013-01-11 15:01:25 +0000
@@ -26,7 +26,8 @@
 #include <cstring>
 
 /// \todo const correctness
-struct RealFSImpl : public FileSystem {
+class RealFSImpl : public FileSystem {
+public:
 	RealFSImpl(std::string const & Directory);
 
 	virtual int32_t FindFiles

=== modified file 'src/io/filesystem/filesystem.h'
--- src/io/filesystem/filesystem.h	2012-06-06 14:51:03 +0000
+++ src/io/filesystem/filesystem.h	2013-01-11 15:01:25 +0000
@@ -42,7 +42,8 @@
  * operations.
  * \todo const correctness
  */
-struct FileSystem {
+class FileSystem {
+public:
 	//  TODO This should be unnecessary. Make it so.
 	enum Type {
 		DIR,

=== modified file 'src/io/filesystem/layered_filesystem.h'
--- src/io/filesystem/layered_filesystem.h	2012-02-15 21:25:34 +0000
+++ src/io/filesystem/layered_filesystem.h	2013-01-11 15:01:25 +0000
@@ -41,7 +41,8 @@
  * $CWD  <-- the current-working directory; this is useful for debugging, when
  * the executable isn't in the root of the game-data directory
  */
-struct LayeredFileSystem : public FileSystem {
+class LayeredFileSystem : public FileSystem {
+public:
 	LayeredFileSystem();
 	virtual ~LayeredFileSystem();
 
@@ -102,7 +103,8 @@
 /// stack and make sure that it is removed when the object goes out of scope.
 /// This is exception-safe, unlike calling g_fs->AddFileSystem and
 /// g_fs->RemoveFileSystem directly.
-struct FileSystemLayer {
+class FileSystemLayer {
+public:
 	FileSystemLayer(FileSystem & fs) : m_fs(fs) {g_fs->AddFileSystem(fs);}
 	~FileSystemLayer() {g_fs->RemoveFileSystem(m_fs);}
 private:

=== modified file 'src/io/filesystem/zip_filesystem.h'
--- src/io/filesystem/zip_filesystem.h	2012-05-05 18:50:50 +0000
+++ src/io/filesystem/zip_filesystem.h	2013-01-11 15:01:25 +0000
@@ -33,7 +33,8 @@
 #endif
 #endif
 
-struct ZipFilesystem : public FileSystem {
+class ZipFilesystem : public FileSystem {
+public:
 	ZipFilesystem(std::string const &);
 	virtual ~ZipFilesystem();
 
@@ -70,7 +71,6 @@
 
 	virtual unsigned long long DiskSpace();
 
-public:
 	static FileSystem * CreateFromDirectory(std::string const & directory);
 
 	virtual std::string getBasename() {return m_zipfilename;};
@@ -81,7 +81,6 @@
 	void m_Close();
 	std::string strip_basename(std::string);
 
-private:
 	enum State {
 		STATE_IDLE,
 		STATE_ZIPPING,

=== modified file 'src/map_io/widelands_map_data_packet.h'
--- src/map_io/widelands_map_data_packet.h	2012-02-15 21:25:34 +0000
+++ src/map_io/widelands_map_data_packet.h	2013-01-11 15:01:25 +0000
@@ -23,7 +23,7 @@
 #include "wexception.h"
 #include "logic/widelands_filewrite.h"
 
-struct FileSystem;
+class FileSystem;
 
 namespace Widelands {
 

=== modified file 'src/map_io/widelands_map_loader.h'
--- src/map_io/widelands_map_loader.h	2012-02-15 21:25:34 +0000
+++ src/map_io/widelands_map_loader.h	2013-01-11 15:01:25 +0000
@@ -24,7 +24,7 @@
 #include <cstring>
 #include "map_loader.h"
 
-struct FileSystem;
+class FileSystem;
 
 namespace Widelands {
 

=== modified file 'src/map_io/widelands_map_object_packet.h'
--- src/map_io/widelands_map_object_packet.h	2012-02-15 21:25:34 +0000
+++ src/map_io/widelands_map_object_packet.h	2013-01-11 15:01:25 +0000
@@ -24,7 +24,7 @@
 
 #include <set>
 
-struct FileSystem;
+class FileSystem;
 
 namespace Widelands {
 

=== modified file 'src/map_io/widelands_map_saver.h'
--- src/map_io/widelands_map_saver.h	2012-02-15 21:25:34 +0000
+++ src/map_io/widelands_map_saver.h	2013-01-11 15:01:25 +0000
@@ -22,7 +22,7 @@
 
 #include "wexception.h"
 
-struct FileSystem;
+class FileSystem;
 
 namespace Widelands {
 

=== modified file 'src/profile/profile.h'
--- src/profile/profile.h	2012-02-15 21:25:34 +0000
+++ src/profile/profile.h	2013-01-11 15:01:25 +0000
@@ -39,7 +39,7 @@
 };
 
 extern struct Profile g_options;
-struct FileSystem;
+class FileSystem;
 
 /**
  * Represents one section inside the .ini-style file, basically as a list of