convert structs to classes

Bug #621861 reported by Monty Taylor on 2010-08-21
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Drizzle
Low
Travis Davies

Bug Description

Drizzle is c++ ... plan structs in the code should be converted to be proper classes.

See Drizzle Blueprint: https://blueprints.launchpad.net/drizzle/+spec/convert-structs-to-classes

Related branches

Changed in drizzle:
assignee: nobody → Travis Davies (travisdaveez)
Travis Davies (travisdaveez) wrote :

.../drizzled/structs.h: Changed C structs KeyPartInfo, KeyInfo, and RegInfo to C++ classes

Travis Davies (travisdaveez) wrote :

Modified File: ../drizzled/ctype-mb.cc at line 1075 converted the static C struct which holds the member utr11_data array, to a C++ class

Travis Davies (travisdaveez) wrote :

Modified the following files:
  drizzled/filesort.cc
  drizzled/filesort_info.h
  drizzled/sql_sort.h
  drizzled/table.h

Travis Davies (travisdaveez) wrote :

modified structs in /drizzled/xid.h to classes

Travis Davies (travisdaveez) wrote :

Converted Structs to Classes in the following files:
File: drizzled/internal/my_sys.h
File: drizzled/internal/my_static.h
File: /plugin/archive/azio.h
File: drizzled/algorithm/sha1.h;

Mohit Srivastava (mohyt) wrote :

Hi Travis
I want to involve in this bug . So where i have to start ??

Mark Atwood (fallenpegasus) wrote :

Hello Mohit. Join the IRC channel and mailing lists, and read the wiki on how to contribute to Drizzle.

The quick answer is to check out the code from launchpad, find a struct in the source code, convert it to a class, test the new code, and then push it back to launchpad and ask that it be merged with the main development truck.

Robin Battey (zanfur) wrote :

In c++, structs *are* classes. The only difference between the keywords is that "struct" declares a class that is by default public, where "class" declares a class that is by default private. They share the same namespace even -- the elaborated-type-specifiers mechanism doesn't apply; they're literally classes.

I have Stroustrup in front of me, 12th printing (May 2005), and on page 234, section 10.2.8, it quoth:

  By definition, a struct is a class in which members are by default public; that is,

    struct s { . . .

  is simple shorthand for

    class s { public: . . .

And this short c++ snippet fails to compile with an actually pretty good error message showing this to be the case:

  zanfur@gandalf:~$ cat foo.cpp
  class foo { int bar; };
  struct foo { int bar; };
  zanfur@gandalf:~$ g++ foo.cpp
  foo.cpp:2: error: redefinition of ‘struct foo’
  foo.cpp:1: error: previous definition of ‘struct foo’

I don't see that this bug is valid, at least as written.

Ivo Roper (ivolucien) wrote :

Thank you Robin, that's an astute and accurate technical observation. I believe that the current bug description relies heavily on this project's context and formal C++ coding culture for effective interpretation.

Based on the intent of Drizzle's refactoring mandate, I interpret this bug as "plain structs in the code should be rewritten as object-oriented classes designed to work with the project's OO design and code standards, including accessor functions and encapsulation of data that can be handled internally (privately) to the class."

Robin Battey (zanfur) wrote :

@Ivo: That's how I took it as well -- hence the comment about "as written".

I believe the bug description would benefit greatly from being updated to the verbiage you just gave, perhaps with a pointer to those design and code standards. Given that this is one of the first bugs (6th one down) I came across from the project homepage, linked to under "Wanna help out?" in the project description, you're going to get a bunch of people who aren't aware of the context.

Apologies if my comment has given offense to anyone. In my background, the way to get bug descriptions changed is to claim they are invalid as written, with support for the claim. That was my attempt here, not to get the bug thrown out.

Ivo Roper (ivolucien) on 2012-03-05
description: updated

converted the structures in the following files to classes: (revision 2536)

  drizzled/field/decimal.cc
  drizzled/function/math/round.cc
  drizzled/internal/my_sys.h
  drizzled/item/cmpfunc.cc
  drizzled/item/decimal.cc
  drizzled/temporal.cc
  drizzled/type/decimal.cc
  drizzled/type/decimal.h

converted the structures in the following files to classes: (revision 2537)

  drizzled/lock.cc
  drizzled/locking/global.cc
  drizzled/session.cc
  drizzled/thr_lock.cc
  drizzled/thr_lock.h
  plugin/innobase/handler/ha_innodb.cc

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Related blueprints