views:

333

answers:

3

Hi, I am learning ASP.NET 3.5 & C#, using Visual Studio 2008. Most of the stuff I learn is through the MSDN. I am trying to develop a web page that will allow a user to create a character for use in an RPG game. The user should be able to allocate attributes, buy items, etc. When the user is done the site will format a printable character sheet with the user data.

Now, I am still wrapping my head around around this stuff and would like to know if I am on the right track - if anyone would like to look at what I have so far and comment that would be awesome. I am interested in anything I am doing wrong or inefficiently, bad design, crappy code, and ways that I can improve. But mostly, I just want to know if I am on the right track and not misusing the technology.

Below is the code I have so far. It allows a user to allocate a certain amount of points to a 4 stats.

Main Page:

<%@ Page Language="C#" 
    AutoEventWireup="true"  
    CodeFile="Default.aspx.cs" 
    Inherits="_Default" 
    enableSessionState="true"
%>


<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"&gt;


<html xmlns="http://www.w3.org/1999/xhtml"&gt;
<head runat="server">
    <title>Untitled Page</title>
</head>
<body>
    <form id="form1" runat="server">
    <div>
        <!-- MPCP/Bod/Evasion/Masking/Sensor -->


        <asp:Label ID="MPCP_Rating" runat="server" Text="" ></asp:Label>
        <br /> <br />
        <asp:Label ID="PersonaPool" runat="server" Text="" ></asp:Label>
        <br /> <br />

        <!-- TODO: Format into table -->
        Bod:
        <asp:TextBox ID="Bod" runat="server" ontextchanged="Bod_TextChanged" 
            width="25px">0</asp:TextBox> 
        <asp:Button ID="BodInc" runat="server" Text="+" 
            OnClick="Bod_Inc" />
        <asp:Button ID="BodDec" runat="server" Text="-"
            OnClick="Bod_Dec"/>
        <br /> <br />

        Evasion:
        <asp:TextBox ID="Evasion" runat="server" ontextchanged="Evasion_TextChanged"
            width="25px">0</asp:TextBox>
        <asp:Button ID="EvasionInc" runat="server" Text="+" 
            OnClick="Evasion_Inc" />
        <asp:Button ID="EvasionDec" runat="server" Text="-" 
            OnClick="Evasion_Dec" /> 
        <br /> <br />

        Masking:
        <asp:TextBox ID="Masking" runat="server" ontextchanged="Masking_TextChanged"
            width="25px">0</asp:TextBox>
        <asp:Button ID="MaskingInc" runat="server" Text="+" 
            OnClick="Masking_Inc" />
        <asp:Button ID="MaskingDec" runat="server" Text="-" 
            OnClick="Masking_Dec" /> 
        <br /> <br />

        Sensor:
        <asp:TextBox ID="Sensor" runat="server" ontextchanged="Sensor_TextChanged"
            width="25px">0</asp:TextBox>
        <asp:Button ID="SensorInc" runat="server" Text="+" 
            OnClick="Sensor_Inc" />
        <asp:Button ID="SensorDec" runat="server" Text="-" 
            OnClick="Sensor_Dec" /> 
        <br /> <br />
        <asp:Button ID="Submit" runat="server" Text="Submit" />

    </div>
    </form>
</body>
</html>

Code-Behind:

using System;
using System.Configuration;
using System.Data;
using System.Linq;
using System.Web;
using System.Web.Security;
using System.Web.UI;
using System.Web.UI.HtmlControls;
using System.Web.UI.WebControls;
using System.Web.UI.WebControls.WebParts;
using System.Xml.Linq;

public partial class _Default : System.Web.UI.Page
{
    private DeckData deck;

    public _Default()
    {
    }

    // Page events
    protected void Page_Load(object sender, EventArgs e)
    {
        deck = (DeckData)(System.Web.HttpContext.Current.Session["Deck"]);
        MPCP_Rating.Text = "MPCP Rating: " + deck.MPCP.ToString();
        UpdateAvailPersona();
    }

    protected void Unload(object sender, EventArgs e)
    {
    }

    // Helper functions
    protected void ChangeAttribute(DeckData.Attributes atr, bool inc)
    {
        if (inc == true) 
            deck.IncAttribute(atr); 
        else  
            deck.DecAttribute(atr); 

        UpdateAvailPersona();

        switch (atr)
        {
            case DeckData.Attributes.Bod:
                Bod.Text = deck.Bod.ToString();
                break;
            case DeckData.Attributes.Evasion:
                Evasion.Text = deck.Evasion.ToString();
                break;
            case DeckData.Attributes.Masking:
                Masking.Text = deck.Masking.ToString();
                break;
            case DeckData.Attributes.Sensor:
                Sensor.Text = deck.Sensor.ToString();
                break;
        }
    }

    protected void UpdateAvailPersona()
    {
        PersonaPool.Text = "Persona Pool: " + deck.PersonaMax.ToString() +
            " / " + (deck.CalculateAvailPersona()).ToString();
    }

    // Control Events 
    protected void Bod_Dec(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Bod, false);
    }

    protected void Bod_Inc(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Bod, true);
    }

    protected void Evasion_Dec(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Evasion, false);
    }

    protected void Evasion_Inc(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Evasion, true);
    }

    protected void Masking_Dec(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Masking, false);
    }

    protected void Masking_Inc(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Masking, true);
    }

    protected void Sensor_Dec(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Sensor, false);
    }

    protected void Sensor_Inc(object sender, EventArgs e)
    {
        ChangeAttribute(DeckData.Attributes.Sensor, true);
    }

App-Data (Just the DeckData class)

using System;
using System.Data;
using System.Configuration;
using System.Linq;
using System.Web;
using System.Web.Security;
using System.Web.UI;
using System.Web.UI.HtmlControls;
using System.Web.UI.WebControls;
using System.Web.UI.WebControls.WebParts;
using System.Xml.Linq;

/// <summary>
/// Store deck related data and provide methods for adjusting deck data
/// </summary>
public class DeckData
{
    // Set persona multiplier, determines max persona pool
    private const uint _PersonaMultiplier = 3;

    public DeckData(uint MPCP)
    {
        _MPCP = MPCP;
        _Bod = _Evasion = _Masking = _Sensor = 0;

        CalculateMaxPersona();
    }

    // MPCP/Bod/Evasion/Masking/Sensor
    public enum Attributes
    {
        MPCP,
        Bod,
        Evasion,
        Masking,
        Sensor
    }

    private uint _MPCP;
    private uint _Bod;
    private uint _Evasion;
    private uint _Masking;
    private uint _Sensor;

    private uint _PersonaMax;

    /// <summary>
    /// Acessor/Modifiers
    /// </summary>
    public uint MPCP
    {
        get { return _MPCP; }
        set { _MPCP = value; }
    }

    public uint Bod
    {
        get { return _Bod; }
        set { _Bod = value; }
    }

    public uint Evasion
    {
        get { return _Evasion; }
        set { _Evasion = value; }
    }

    public uint Masking
    {
        get { return _Masking; }
        set { _Masking = value; }
    }

    public uint Sensor
    {
        get { return _Sensor; }
        set { _Sensor = value; }
    }

    public uint PersonaMax
    {
        get { return _PersonaMax; }
    }

    /// <summary>
    /// Calculate available persona. Must be called before changing attribs to ensure
    /// persona pool remains valid
    /// </summary>
    /// <returns></returns>
    public uint CalculateAvailPersona()
    {
        // Total deck attribs
        uint attrTotal = _Bod + _Evasion + _Masking + _Sensor;

        return _PersonaMax - attrTotal;
    }

    /// <summary>
    /// Recalculate max persona
    /// </summary>
    private uint CalculateMaxPersona()
    {
        _PersonaMax = _MPCP * _PersonaMultiplier;
        return _PersonaMax;
    }

    /// <summary>
    /// Increment attribute by 1 point 
    /// </summary>
    /// <param name="atr">
    /// The attribute to increment
    /// </param>
    /// <returns>
    /// false if no Persona available
    /// true if attribute successfully incremented
    /// </returns>
    public bool DecAttribute(DeckData.Attributes atr)
    {
        uint availPersona = CalculateAvailPersona();

        if (availPersona == _PersonaMax)
            return false;

        switch (atr)
        {
            case Attributes.MPCP:
                break;
            case Attributes.Bod:
                if (_Bod > 0)               // Check for underflow
                    _Bod -= 1;
                break;
            case Attributes.Evasion:
                if (_Evasion > 0)
                    _Evasion -= 1;
                break;
            case Attributes.Masking:
                if (_Masking > 0)
                    _Masking -= 1;
                break;
            case Attributes.Sensor:
                if (Sensor > 0)
                    _Sensor -= 1;
                break;
        }

        // Check to see if we updated an attribute using cached persona
        if(availPersona != CalculateAvailPersona())
            return true;
        return false;
    }

    public bool IncAttribute(DeckData.Attributes atr)
    {
        uint availPersona = CalculateAvailPersona();

        if (availPersona == 0)
            return false;

        switch (atr)
        {
            case Attributes.MPCP:
                break;
            case Attributes.Bod:
                _Bod += 1;
                break;
            case Attributes.Evasion:
                _Evasion += 1;
                break;
            case Attributes.Masking:
                _Masking += 1;
                break;
            case Attributes.Sensor:
                _Sensor += 1;
                break;
        }

        return true;
    }
}

Thanks!

+3  A: 

If you are good with html, give up webforms and use an mvc framework like asp.net mvc or fubu mvc.

If you are not good with html, learn html, give up webforms and use an mvc framework.

Serhat Özgel
I'm not sure this is the best advice. Web forms are still hugely prevalent. I don't see MVC getting penetration until a couple of years down the line. This is only applicable to ASP.NET - PHP mvc frameworks are everywhere!
Rob Stevenson-Leggett
Cool, that clears some things up for me. I am interested in learning PHP as well. I suppose that would be the best place to start researching MVC frameworks.
I feel that the MVC framework should not be underestimated here...Because the knowledge you can get from it will also be usable in other frameworks. If asp.net-mvc never gets it's foot through the door you will still be able to use it in the other MVC frameworks.
borisCallens
Agreed with Boris. Learning WebForms will cage you in ASP.NET world, while learning MVC will help you get familiar with other frameworks, such as Rails. Besides you will master HMTL and JavaScript. :)
Pawel Krakowiak
Rails looks pretty sexy :)
+1  A: 

I'v just had a quick look at you code and one thing that does leap out is the amount of server postbacks that would be involved in using your web app. Every change to an attribute will result in the page being submitted back to the server which, whilst technically is o.k., does not make for a great user experience.

You may want to investigate techniques that will allow the user to make all their modifications and then submit them all together (such as using javascript on the client side to maintain the distribution of attribute points) or alternatively look into using AJAX to asynchronously post back the modifications to the server resulting in a smoother user experience.

EDIT: With the model you are using you could use the Button controls Command event instead of the Click event. This will allow you to assign a CommandName and CommandArgument value to each button which can be recovered in the code behind. This will allow you to have just the one event method for each button which can decided which attribute to change and how from these properties:

<asp:Button ID="BodInc" runat="server" CommandArgument="Increase" 
        CommandName="Bod" oncommand="AttributeButton_Command" Text="+" />
<asp:Button ID="BodDec" runat="server" CommandArgument="Increase" 
        CommandName="Bod" oncommand="AttributeButton_Command" Text="-" />

code behind:

protected void AttributeButton_Command(object sender, CommandEventArgs e)
{
    string attriubuteName = e.CommandName;
    string action = e.CommandArgument;
    // Do stuff
}
Andy Rose
Excellent idea, thank you very much :)
A: 

One suggestion that I would make is to look at using javascript to increment/decrement your counters in your markup, then do the updates to your model using the values of the text boxes on form submission. As @Andy suggests, you could also do the updates via AJAX to reduce visible UI flickering, but given the simple rules you have I think doing it client-side and posting back once is the way to go.

Unless you want the user to enter numeric values directly, I'd go for a static display of the chosen number and enforce use of the up/down buttons. You could make this look like a combination lock that only allows up/down selections if the sum of the currently chosen values is less than the amount available. Of course, you'll need to update the code to validate that the maximums aren't being exceeded. Disallowing direct user input will save you the trouble of validating that only numeric values are entered.

Bod      + [0] =
Evasion  + [0] -
Masking  + [0] -
Sensor   + [0] -
Total       0
Maximum     ?

You also want to think about styling using CSS classes rather than specifying widths directly in your markup. Using CSS will help you keep a consistent look and feel throughout your application and allow you to change that look and feel later quickly and easily by (for the most part) changes to your CSS and not your application code itself.

tvanfosson