Language: Java
Description: Add or remove a bot members access flags.

This is being posted to help anyone interested, but any improvement posts regarding my code are welcome. If you do not program in Java, convert it. ;D

[code]
   private StringBuffer access_flags; // Member access flags
   //--------------------------------------------------------------------------
   //- Parse and set the members access flags.
   //--------------------------------------------------------------------------
   public void setAccessFlags (String flags)
   {
      boolean add = false;
      String allowed = "+-nfeuom";
      
      flags = flags.toLowerCase();
      
      for (int i = 0; i < flags.length(); i++)
      {
         // Ignore any unused characters
         if (allowed.indexOf (String.valueOf(flags.charAt (i))) != -1)
            if (flags.charAt (i) == '+')
               add = true;
            else if (flags.charAt (i) == '-')
               add = false;
            else
               if (add)
               {
                  // Make sure flag not already set
                  if (access_flags.indexOf (String.valueOf (
                        flags.charAt (i))) == -1)
                     access_flags.append (flags.charAt (i));
               }
               else
               {
                  // Make sure flag is already set
                  if (access_flags.indexOf (String.valueOf (
                        flags.charAt (i))) != -1)
                     access_flags.deleteCharAt (access_flags.indexOf (
                                 String.valueOf (flags.charAt(i))));
               }
      }
   }
[/code]

Actually, you shouldn't actually store the letters for the flags. A bitfield, bunch of bools, or something similar would be much better.

Didn't even think of that, thanks.

This union by Skywing is a nice example of how to store, set, add, and remove flags.

[code]
// User attribute data type (32 bits)
union UserAttributes {
   struct {
      // Privileges
      unsigned A : 1;
      unsigned B : 1;
      unsigned C : 1;
      unsigned D : 1;
      unsigned E : 1;
      unsigned F : 1;
      unsigned G : 1;
      unsigned H : 1;
      unsigned I : 1;
      unsigned J : 1;
      unsigned K : 1;
      unsigned L : 1;
      unsigned M : 1;
      unsigned N : 1;
      unsigned O : 1;
      unsigned P : 1;
      unsigned Q : 1;
      unsigned R : 1;
      unsigned S : 1;
      unsigned T : 1;
      unsigned U : 1;
      unsigned V : 1;
      unsigned W : 1;
      unsigned X : 1;
      unsigned Y : 1;
      unsigned Z : 1; // 26
      // Other information
      unsigned Designated : 1;
      unsigned Operator : 1; // 28
      unsigned Reserved : 4; // 32
   };
   struct {
      unsigned Attributes : 26; // 26
      unsigned Information : 6; // 32
   };
   inline void Set(unsigned char cAttribute)
   {
      if(cAttribute < 'A' || cAttribute > 'Z')
         return;
      dwFlags |= (1 << (cAttribute - 'A'));
   }
   inline void Set(UserAttributes _Attributes)
   {
      Attributes |= _Attributes.Attributes;
   }
   inline void Remove(unsigned char cAttribute)
   {
      if(cAttribute < 'A' || cAttribute > 'Z')
         return;
      dwFlags &= ~(1 << (cAttribute - 'A'));
   }
   inline void Remove(UserAttributes _Attributes)
   {
      Attributes &= ~_Attributes.Attributes;
   }
   inline bool Check(unsigned char cAttribute) const
   {
      if(cAttribute < 'A' || cAttribute > 'Z')
         return false;
      return !!(dwFlags & (1 << (cAttribute - 'A'))); // ~~~ not 'a' gddmit ~~~
   }
   inline bool Check(UserAttributes _Attributes) const
   {
      return (Attributes & _Attributes.Attributes) == _Attributes.Attributes;
   }
   inline void Replace(UserAttributes _Attributes, bool bReplaceInformation)
   {
      if(bReplaceInformation)
         dwFlags = _Attributes.dwFlags;
      else
         Attributes = _Attributes.Attributes;
   }
   DWORD dwFlags; // 32
};
[/code]

Thanks for the source it is a great example.

I am using this as more of a learning exercise though. Instead of implementing someone else's solution I am trying to create my own to the best of my knowledge. I will be the first to admit that while I do have a good understanding of a particular programming languages syntax and common libraries (Java), I am a complete beginner when it comes to programming theories. So the idea behind this thread, and more to come, is that myself and anyone else interested come up with the best results based on debates and studies we perform ourselves.

Storm made a good response, and I am going to use that to better my current project. Then I am going to post my results for anyone else interested. So all I am asking is while in these threads please post useful responses that will help anyone interested, and keep everyone else's source alone. To all the experts, please refrain from flaming, and to the beginners (How do I make a bot?), if you are truly interested, feel free to use these thread's ideas to help yourself answer this question.

Remember that because Java suckily does not implement unsigned types, normal right shift will be a sar, rather than shr. For negative types, this means the value's sign bit is copied downward in Java (so if your uppermost bit has meaning and you have it set and you go rightshifting things for analysis...). To avoid this problem, use >>> instead of >>.

It also preserves the sign extension (leftmost bit). For example -8 when shifted to the right 1 time will be -4. In Java this is done to preserve negative numbers. Just read up on your languages implementation of the bitwise operators, and types available to you.

[code]
//******************************************************************************
//* AccessFlags.java            Author: Tuberload
//*
//*    Handles bot members access flags.
//******************************************************************************
package tuberload.database;
import java.util.Hashtable;
//******************************************************************************
public class AccessFlags
{
   private Hashtable allowed_flags;
   //--------------------------------------------------------------------------
   //- Initialize class.
   //--------------------------------------------------------------------------
   public AccessFlags()
   {
      allowed_flags = new Hashtable();
      allowed_flags.put ("n", new Integer (0x1));
      allowed_flags.put ("f", new Integer (0x2));
      allowed_flags.put ("e", new Integer (0x4));
      allowed_flags.put ("u", new Integer (0x8));
      allowed_flags.put ("o", new Integer (0x10));
      allowed_flags.put ("p", new Integer (0x20));
   }
   //--------------------------------------------------------------------------
   //- Add and remove user flags.
   //--------------------------------------------------------------------------
   public int modify (int access_flags, String flags)
   {
      flags = flags.toLowerCase();
      
      for (int i = 0; i < flags.length(); i++)
      {
         if (allowed(flags.charAt (i)))
            access_flags = setFlags (flags.charAt (i), access_flags);
      }
      return access_flags;
   }
   //--------------------------------------------------------------------------
   //- Verifies if a character is allowed or not. Must be a proper access flag
   //- to pass.
   //--------------------------------------------------------------------------
   private boolean allowed (char flag)
   {
      boolean allow = false;
      
      if (allowed_flags.containsKey(String.valueOf (flag)))
         allow = true;
      
      return allow;
   }
   //--------------------------------------------------------------------------
   //- Set the access flags,
   //--------------------------------------------------------------------------
   private int setFlags (char flag, int access_flags)
   {
      Integer mask = (Integer)allowed_flags.get(String.valueOf (flag));
      return access_flags ^= mask.intValue();
   }
}[/code]

Here is an update to my previous code. It is dramatically different, I know, but way more efficient. There is a lot more that can be added to it, but this should help anyone interested. It is set up so that in the future I can allow users to add flags to the system by using a Hashtable to store the flag/mask combinations.

For those interested in what is going on, here is an explanation. It Loops through the flags string checking to see if each character is a valid flag or not. If a valid flag is found it XOR's the access_flags integer with the corresponding bitmask. By XORing the access_flags integer I can turn bits on that are off, and bits off that are on. For Example: If you want a user to have the "fuop" flags, his corresponding access_flags value will be (binary string) 111010 or 58. Then lets say you want to remove the "o" flag. His new access_flags value will be 101010 or 42.

Once again, if anyone sees anything wrong with my code, please let me know.

Another thing to keep in mind is that the code above no longer recognizes the '+' or '-' characters. So it will turn the flags you specify either on or off automatically. If this is an issue for you, feel free to compensate accordingly.

[code]
//--------------------------------------------------------------------------
//- Find out if a flag is set or not.
//--------------------------------------------------------------------------
public boolean isSet (int access_flags, char flag)
{
boolean is_set = false;
Integer mask = (Integer)allowed_flags.get(String.valueOf (flag));

if ((access_flags & mask.intValue()) == mask.intValue())
is_set = true;
      
return is_set;
}
[/code]

Here is an important method I forgot to add in before posting.

Some suggested modifications:
[code]
private boolean allowed (char flag)
{
return allowed_flags.containsKey(String.valueOf (flag));
}
private int toggleFlags (char flag, int access_flags)
{
return access_flags ^ ((Integer)allowed_flags.get(String.valueOf (flag))).intValue();
}
}[/code]Modified allowed(char) not to bother with a temporary and branch, since it just returns the truth of whether your hashmap contains the flag. Renamed setFlags(char,int) to toggleFlags(char,int) to reflect that it xors them, not assigns them.

Possible future modifications:
* Add a getFlags(char) function that takes a flag as a character and returns the corresponding integer value. This could then be used as a primitive for addFlags(), delFlags(), etc.
* Add addFlags() and delFlags(), to enable and disable, respectively, a flag, without regard for its previous state. Possibly have the function return a boolean indicating whether the operation actually caused a change (false indicates that the flag was already in the specified state, true that the function changed it).
* Add handling for bogus flags. I haven't checked the Java reference, but from what I recall of Java, I doubt your code would survive someone passing a bad flag in (hashtable will probably either throw an exception or return a null, which you then cast to Integer -- either way, bad). Your present check with the allowed() function handles this for now, but it might be faster to just let the failure happen and catch it if/when it does, rather than ensure that it won't, then doing the work.

[quote]* Add handling for bogus flags. I haven't checked the Java reference, but from what I recall of Java, I doubt your code would survive someone passing a bad flag in (hashtable will probably either throw an exception or return a null, which you then cast to Integer -- either way, bad). Your present check with the allowed() function handles this for now, but it might be faster to just let the failure happen and catch it if/when it does, rather than ensure that it won't, then doing the work. [/quote]

It does return a null if the flag is not found. So I tried a new method as you suggested. I did away with the allowed() method call, and had the toggleFlags() method check to see if a null is returned and return either the modified access_flags or an unmodified flag value. Here is the new toggleFlags() method:
[code]
   public int modify (int access_flags, String flags)
   {
      flags = flags.toLowerCase();
      
      for (int i = 0; i < flags.length(); i++)
      {
         //if (allowed(flags.charAt (i)))
         access_flags = toggleFlags (flags.charAt (i), access_flags);
      }
      return access_flags;
   }
   private int toggleFlags (char flag, int access_flags)
   {
      // If flag is found return modified access_flags
      return allowed_flags.get (String.valueOf (flag)) != null ? access_flags
               ^ ((Integer)allowed_flags.get(String.valueOf (flag))).intValue() : access_flags;
      /*return access_flags ^ ((Integer)allowed_flags.get(String.valueOf (
                                             flag))).intValue();*/
   }
[/code]
I then tested the two, and found the previous way I was doing it took 100 milliseconds, and this new method took 111. My test code toggles all available flags, plus one that is not available 10,000 times. My test code is as follows:
[code]
   AccessFlags test = new AccessFlags();
   int flag = 0x1; // initially set to 'n'
   long time = System.currentTimeMillis();
   
   for (int i = 0; i < 10000; i++)
      flag = test.modify (flag, "nfeuomz");
         
   long time2 = System.currentTimeMillis() - time;
   System.out.println (time2);
[/code]
As it stands my previous method is faster, although I am sure you, Kp, can point out some areas of interest. I tried to go with your idea of letting the modification take place without an initial check, and then just handling any unwanted returns.

Thank you very much Kp, you have been a tremendous help to me. You have made me think a lot, and it has benefited me greatly in my entire project.

I also tried placing the toggleFlags() method in a try/catch statement to handle the NullPointerException that would be thrown on the event of a bogus flag.
[code]
   try
   {
      access_flags ^= ((Integer)allowed_flags.get(String.valueOf (
                                             flag))).intValue();
   }
   catch (NullPointerException exc) {}
   return access_flags;
[/code]
The time results were 430 milliseconds. Maybe using Java's try/catch statements is not be a good idea for the sake of performance.

We anaylsed that idea quite alot during one of my Comp Sci papers and in terms of performance its not too flash but for cleaness of code and structure its pretty good. I doubt your really going to achieve a substantial improvement by using anything else. Its just a nice way to deal with all possible Exceptions.

[quote author=Orillion link=board=17;threadid=3473;start=0#msg28385 date=1068426936]
We anaylsed that idea quite alot during one of my Comp Sci papers and in terms of performance its not too flash but for cleaness of code and structure its pretty good. I doubt your really going to achieve a substantial improvement by using anything else. Its just a nice way to deal with all possible Exceptions.
[/quote]

I am assuming that you are talking about the try/catch statements, and using them, based on my tests, is 4x slower than not allowing a bogus flag to be passed at all. I always thought parameter checking was clean/structured code. I do not see the benefits of using a try/catch statement in this case because I do not need to do anything in the event of a bogus flag being passed, plus it is so much slower. The only other use that could come out of a try/catch would be to throw the exception to the user of the class, which I also don't see as beneficial.

Seems like it'd be more intuitive to work with operator overloads instead of functions like Set/UnSet/etc.

[quote author=Eibro link=board=17;threadid=3473;start=15#msg28389 date=1068427860]
Seems like it'd be more intuitive to work with operator overloads instead of functions like Set/UnSet/etc.
[/quote]
I don't understand how it would be more intuitive, but Java does not support operator overloading that I am aware of. I think using Accessor/Mutator methods for setting and getting data works fine, even though I don't even see what Set/Unset methods you are talking about.

[quote author=Tuberload link=board=17;threadid=3473;start=0#msg28361 date=1068421783]
It does return a null if the flag is not found. So I tried a new method as you suggested. I did away with the allowed() method call, and had the toggleFlags() method check to see if a null is returned and return either the modified access_flags or an unmodified flag value. Here is the new toggleFlags() method:
[code]
public int modify (int access_flags, String flags)
{
flags = flags.toLowerCase();

for (int i = 0; i < flags.length(); i++)
{
//if (allowed(flags.charAt (i)))
access_flags = toggleFlags (flags.charAt (i), access_flags);
}
return access_flags;
}
private int toggleFlags (char flag, int access_flags)
{
// If flag is found return modified access_flags
return allowed_flags.get (String.valueOf (flag)) != null ? access_flags
^ ((Integer)allowed_flags.get(String.valueOf (flag))).intValue() : access_flags;
/*return access_flags ^ ((Integer)allowed_flags.get(String.valueOf (
flag))).intValue();*/
}
[/code]
I then tested the two, and found the previous way I was doing it took 100 milliseconds, and this new method took 111. My test code toggles all available flags, plus one that is not available 10,000 times. My test code is as follows:
[code]
AccessFlags test = new AccessFlags();
int flag = 0x1; // initially set to 'n'
long time = System.currentTimeMillis();

for (int i = 0; i < 10000; i++)
flag = test.modify (flag, "nfeuomz");

long time2 = System.currentTimeMillis() - time;
System.out.println (time2);
[/code]
As it stands my previous method is faster, although I am sure you, Kp, can point out some areas of interest. I tried to go with your idea of letting the modification take place without an initial check, and then just handling any unwanted returns.[/quote]Well, some of the performance hit might be coming from multiply querying the hash table. Try using this for your toggle method:
[code]
private int toggleFlags (char flag, int access_flags)
{
Object o = allowed_flags.get (String.valueOf (flag));
if (o != null)
access_flags ^= ((Integer) o).intValue ();
return access_flags;
}
[/code]
This way, you perform only one lookup, and one class casting checking. If the lookup fails (null return), you skip the conditional and return without doing any additional work. Note that the cast to Integer cannot be performed until after it's confirmed that the returned Object was not null. Otherwise, I'd have cast the return immediately and stored it in an Integer reference for readability's sake.

[quote author=Tuberload link=board=17;threadid=3473;start=0#msg28363 date=1068422392]
I also tried placing the toggleFlags() method in a try/catch statement to handle the NullPointerException that would be thrown on the event of a bogus flag.
[code]
try
{
access_flags ^= ((Integer)allowed_flags.get(String.valueOf (
flag))).intValue();
}
catch (NullPointerException exc) {}
return access_flags;
[/code]
The time results were 430 milliseconds. Maybe using Java's try/catch statements is not be a good idea for the sake of performance.
[/quote]Yes; I knew that in C++, use of exceptions tended to be much slower than developer-implemented error checking. I'd hoped that it wouldn't be as bad in Java, since they had a chance to implement exceptions directly into the VM. Apparently not. :P

After writing BeefBot, a high-performace, plugin-oriented (and horribly unreliable due to a shitty reactor implementation) Java bot, I did some research into the best way to implement a user database.

Basically, you're going to want to have a hashmap of hashmaps. The top hashmap is one keying lowercase usernames to their hashmap of properties. Perhaps this is overkill for your application; you may want to use a bitfield/custom class. I chose a hashmap because I stored lots of information. After changing a user, mark them as dirty. Every few minutes have a daemon write it to disk.

The "best" way to do this is a BTree...but I don't have time to create a good implementation :)

You may investigate SQLLite...but I know you really want to write it yourself, don't you?

[quote author=St0rm.iD link=board=17;threadid=3473;start=15#msg28707 date=1068582264]
After writing BeefBot, a high-performance, plugin-oriented (and horribly unreliable due to a shitty reactor implementation) Java bot, I did some research into the best way to implement a user database.

Basically, you're going to want to have a hashmap of hashmaps. The top hashmap is one keying lowercase usernames to their hashmap of properties. Perhaps this is overkill for your application; you may want to use a bitfield/custom class. I chose a hashmap because I stored lots of information. After changing a user, mark them as dirty. Every few minutes have a daemon write it to disk.

The "best" way to do this is a BTree...but I don't have time to create a good implementation :)

You may investigate SQLLite...but I know you really want to write it yourself, don't you?
[/quote]

Yes, I know that I am re-inventing the wheel, but I do like to do it myself. :) It helps me to learn the language better when I do it myself and I don’t have time constraints. This was however a small part of the bot I am creating and I had hoped it would help others, while helping myself. I do however use Hashtables extensively in my bot to store user information though. I create a BotMember class that stores all the information, then store it in a Hashtable. I serialize the BotMember classes when a member leaves the channel and deserialize them once on join/user events to save on memory at the sake of performance. If the local database does not contain a previously saved member I just create a new one.

Kp: I use the ternary (Sp?) operator to do the exact thing you do don’t I? It checks to see if the value is null and returns the original flags if it is, otherwise it returns the modified flags. If I am missing something please let me know, but I don’t see the difference between my code and yours except how it was done. Never mind I see what you are saying, stupid me. In my drive to make the method as small as possible I did create added overhead. Thank you once again for your help.

[quote author=Tuberload link=board=17;threadid=3473;start=15#msg31973 date=1069929033]
If the local database does not contain a previously saved member I just create a new one.[/quote]

I understand the value of this, and once did it myself. However, I discontinued the practice a couple years ago, for the following reason: one-shot visitors. If you hang out in a popular channel (e.g. Op [vL], or any other place where people just stop by for a bit), you'll get a lot of entries created from one-shot visitors who never return, or at least weren't interesting enough to merit remembering. Additionally, massbots and certain types of floodbots can be very damaging if you create an entry for every name you see them use... It's much better to only create a user entry when an authorized user directs that it be done. That will (mostly) cut down on bogus / garbage db entries.

[quote author=Tuberload link=board=17;threadid=3473;start=15#msg31973 date=1069929033]
Kp: I use the ternary (Sp?) operator to do the exact thing you do don’t I? It checks to see if the value is null and returns the original flags if it is, otherwise it returns the modified flags. If I am missing something please let me know, but I don’t see the difference between my code and yours except how it was done. Never mind I see what you are saying, stupid me. In my drive to make the method as small as possible I did create added overhead. Thank you once again for your help.
[/quote]

:) It's unfortunate that the compiler isn't quite smart enough to be able to optimize your code to look like mine automatically. I suspect it could, if Java had a method to declare methods as being 'const' with respect to their objects. *cough* C++ can do this, Java can't -- the only way Java can create const objects is by having the object artificially enforce that, such as by not having any public mutator methods.

[Edit: minor wording change.]